Conversation
Tests are not mandatory, but if you want to add some you'll need to basically create a dummy project in which to run the project detection commands. There are plenty of similar tests already. I'll have to take a closer look at the code, but overall the PR seems reasonable to me. |
| (require 'compile) | ||
| (require 'grep) | ||
| (require 'lisp-mnt) | ||
| (require 'xml) |
There was a problem hiding this comment.
I'd suggest moving this in the function that uses it, so we don't slow down loading Projectile needlessly.
| "Command used by projectile to get the files in a svn project." | ||
| :group 'projectile | ||
| :type 'string) | ||
| (defcustom projectile-osc-command #'projectile-osc-command-files |
There was a problem hiding this comment.
Please add a blank line before this.
| (defcustom projectile-osc-command #'projectile-osc-command-files | ||
| "Command used by projectile to get the files in a svn project." | ||
| :group 'projectile | ||
| :type '(function string)) |
There was a problem hiding this comment.
Don't forget to add a :package-version property.
| (defun projectile-osc-command-files (&optional root) | ||
| "Return files of osc vcs, return either packages or files belonging to package." | ||
| (or root (setq root default-directory)) | ||
| (let* ((files_ (car (xml-parse-file (if (file-exists-p ".osc/_files") |
There was a problem hiding this comment.
It seems to me a cond might be a better fit than the nested ifs you've opted for.
There was a problem hiding this comment.
Why
files_instead offiles?
Because the file containing the content is called _files and _files as variable name didn't exactly work.
There was a problem hiding this comment.
Will check using cond for this.
| (when (or (stringp command) | ||
| (functionp command)) | ||
| (let ((default-directory root)) | ||
| (if (functionp command) |
There was a problem hiding this comment.
I'm puzzled by this code, as you call the command, but don't do anything with the result.
There was a problem hiding this comment.
I do something with the result. The external command if a function is called, returns the list of files and that variable is returned.
|
Let me know when you feel this is ready for another review. |
Thaodan
force-pushed
the
scm_osc
branch
2 times, most recently
from
a700dd9 to
8d24fac
Compare
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed soon if no further activity occurs. Thank you for your contribution and understanding! |
Signed-off-by: Björn Bidar <bjorn.bidar@thaodan.de>
2 participants
Support the OSC SCM as used in the open build service.
OSC can be used with vcs-osc in Emacs.
The code is wip but I wanted to post this PR to get some feedback
and show progress working on it.
Osc works a little different than other scms where there two different
types of repositories, one containing packages and the other being a package.
The type that is package works very much like a regular scm where commands
such as those to open project files work as a user would expect.
But for the project type the commands referring to files target the packages the project
contains.
I've implemented the functionality using the builtin XML functionality as that is the easiest
since osc's own files use XML.
I'm not exactly sure how to add test, any help is appreciated.
Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test)M-x checkdocwarnings