Skip to content

Comments

yupdate - allow building the web frontend in the development mode#404

Merged
lslezak merged 1 commit intomasterfrom
yupdate_mode
Jan 20, 2023
Merged

yupdate - allow building the web frontend in the development mode#404
lslezak merged 1 commit intomasterfrom
yupdate_mode

Conversation

@lslezak
Copy link
Contributor

@lslezak lslezak commented Jan 20, 2023

Problem

The yupdate script builds the web frontend in the production mode. That has some disadvantages:

  • The debugging files (*.map) are missing, you cannot get a reference to the original sources, this makes debugging more difficult
  • The build takes longer time

Solution

  • Allow reading the NODE_ENV value from the current environment

Testing

  • Tested manually with NODE_ENV=development yupdate ...

@coveralls
Copy link

Coverage Status

Coverage: 75.743%. Remained the same when pulling f76b2f8 on yupdate_mode into 5c90b07 on master.

Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Looks good, just an idea commented there. Thank you!

Comment on lines +121 to +129
# clean up the extra files when switching the development/production mode
if node_env == "production"
# remove the uncompressed and development files
FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/index.{css,html,js}"))
FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/*.map"))
else
# remove the compressed files
FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/*.gz"))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

NP: Why not just removing the full content before running the make install?

Anyway, it looks good, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If make install failed you would have empty directory, i.e. no d-installer at all, this is safer.

@lslezak lslezak merged commit ef8d8e2 into master Jan 20, 2023
@lslezak lslezak deleted the yupdate_mode branch January 20, 2023 13:27
@lslezak lslezak mentioned this pull request Jan 20, 2023
@imobachgs imobachgs mentioned this pull request Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants