Skip to content

ENT-12600: compile: Documented script #1775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

larsewi
Copy link
Contributor

@larsewi larsewi commented Jun 24, 2025

  • compile: fixed shellcheck issues
  • compile: Reduced verbosity of script
  • compile: Fixed inconsistent indents
  • compile: Placed block terminator on a new line for consistency
  • compile: Documented script

Build with no tests
Build Status

Example output:

$ PROJECT=nova ROLE=hub ./buildscripts/build-scripts/compile
 ---snip---
compile: Debug: Running make in core repo...
compile: Debug: Running make install in core repo...
compile: Debug: Running make in enterprise repo...
compile: Debug: Running make install in enterprise repo...
compile: Debug: Running make in nova repo...
compile: Debug: Running make install in nova repo...
compile: Debug: Running make install in masterfiles repo...

^ Snipped noise from sourced scripts

Ticket: ENT-12600
Signed-off-by: Lars Erik Wik <[email protected]>

. `dirname "$0"`/functions
# Usage: PROJECT=[nova|community] ROLE=[hub|agent] ./buildscripts/build-scripts/compile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but we should probably align this with the names we use elsewhere, i.e.
enterprise instead of nova and client instead of agent.

Copy link
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but nice if @craigcomstock can review as well.

Comment on lines +13 to +14
# ├── enterprise
# ├── nova
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here, as in another script I reviewed you might include two blocks: one for community, the other for ent.

Comment on lines +40 to +43
echo "$(basename "$0"): Debug: Running make in enterprise repo..."
run_and_print_on_failure $MAKE -C "$BASEDIR"/enterprise -k
echo "$(basename "$0"): Debug: Running make install in enterprise repo..."
run_and_print_on_failure $MAKE -C "$BASEDIR"/enterprise install DESTDIR="$BASEDIR"/cfengine/dist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting to break it up separate for nova and the -k for keep going. I suppose these are reasonable but I wonder if just make install wouldn't make more sense. We should get about the same output. The advantage of -k is we get more of the problems with each run. I never use that option as I guess I would prefer to see the first error, fix it, repeat. Also if -j is anything but 1 I'm not sure if -k really buys us much in terms of readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants