Add bazel-build-bazel-command - #16
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Sometimes users have custom invocation scripts that eliminate the need for a user to install bazel to build their project. This allow `bazel-build` to use that invocation script directly.
d837392 to
fbae714
Compare
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| :group 'languages) | ||
|
|
||
| (defcustom bazel-build-bazel-program "bazel" | ||
| "Name of the 'bazel' program for 'bazel-build' and 'bazel-test'." |
There was a problem hiding this comment.
Nit: Quote Lisp symbols in docstrings `like-this' or ‘like-this’. See https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html.
There was a problem hiding this comment.
Thanks, new to this part of elisp will fix this in a bit.
There was a problem hiding this comment.
SG, thanks, please ping this thread once you're ready.
|
Laurent, is this ready to merge? |
|
The last reviewer comment was:
ping @jlisee |
|
I have a patch that does essentially the same thing. Can we merge this now, and fix the nit later? |
|
@phst Shall we merge this? |
Yes, that's fine with me. We can fix the nits afterwards. |
|
I can't merge now, there are conflicts. @jlisee, could you update your branch? |
|
|
||
| (defun bazel-build (target) | ||
| "Build a Bazel TARGET." | ||
| (interactive (list (bazel-build--read-target "bazel build "))) |
There was a problem hiding this comment.
It would be good to also have the prompts here use the the variable instead of hard-coding "bazel".
| "Name of the 'bazel' program for 'bazel-build' and 'bazel-test'." | ||
| :type 'string | ||
| :group 'bazel-build | ||
| :link '(url-link "https://bazel.build/")) |
There was a problem hiding this comment.
Since this is the kind of thing that is often project local (i.e., in .dir-locals.el), it'd be useful to add a :safe field.
From the Emacs info page:
When defining a user option using ‘defcustom’, you can set its
‘safe-local-variable’ property by adding the arguments ‘:safe FUNCTION’
to ‘defcustom’ (*note Variable Definitions::). However, a safety
predicate defined using ‘:safe’ will only be known once the package that
contains the ‘defcustom’ is loaded, which is often too late. As an
alternative, you can use the autoload cookie (*note Autoload::) to
assign the option its safety predicate, like this:
;;;###autoload (put 'VAR 'safe-local-variable 'PRED)
The safe value definitions specified with ‘autoload’ are copied into the
package’s autoloads file (‘loaddefs.el’ for most packages bundled with
Emacs), and are known to Emacs since the beginning of a session.
Sometimes users have custom invocation scripts that eliminate the need
for a user to install bazel to build their project. This allow
bazel-buildto use that invocation script directly.