[GitHub] [buildstream-plugins] gtristan commented on a diff in pull request #27: Have absolute path for conf-root

[GitBox <gi...@apache.org>]

gtristan commented on code in PR #27:
URL: https://github.com/apache/buildstream-plugins/pull/27#discussion_r949835243


##########
src/buildstream_plugins/elements/autotools.yaml:
##########
@@ -17,6 +17,11 @@ variables:
   # Project-wide extra arguments to be passed to `configure`
   conf-global: ''
 
+  # This needs to be overridden for build-dir to work

Review Comment:
   Two things here:
   * The sentence *"This needs to be overridden for build-dir to work command-subdir is for monorepos to work"* doesn't read very well and appears to be developer facing technical detail. As this yaml is rendered in [user facing documentation](https://apache.github.io/buildstream-plugins/elements/autotools.html), comments here should be user facing, e.g.:
     * `conf-root`: The directory where the configure script will be generated and used
     * `command-subdir`: The subdirectory of `%{build-root}` where commands will be run
   * I think this should be positioned near the `build-dir` as they seem functionally related
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org