Skip to content
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.

Sign up for GitHub

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

Jump to bottom

make ready for rspack #26

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

make ready for rspack #26

wants to merge 4 commits into from

Conversation

utnapischtim
Copy link
Contributor

  • context: make the bundle current_app unaware
  • pnpm: add override for pnpm
  • project: remove current_app usage

@utnapischtim utnapischtim mentioned this pull request Jan 23, 2025
Draft
@utnapischtim utnapischtim marked this pull request as ready for review February 20, 2025 08:10
@utnapischtim
context: make the bundle current_app unaware
4df8d8f
@utnapischtim
project: add configuration for js package manager
da488a0 
* it is possible to choose between npm and pnpm over the configuration
  variable JAVASCRIPT_PACKAGES_MANAGER. to be backward compatible npm is
  the default package manager.
@utnapischtim
project: sneak in app
af8dbb8 
* this makes it possible to reduce usage of current_app and enables
  performance boosts on invenio-cli
@max-moser max-moser mentioned this pull request Mar 1, 2025
Draft
@utnapischtim
Copy link
Contributor Author

utnapischtim commented Mar 1, 2025 •
edited
Loading

the change to use app instead of current_app was made to use the classes in invenio-cli directly. The approach of using the classes directly had a massive drawback (invenio-cli was not usable if installed globally). so invenio-cli has been changed. a new approach has been implemented here which doesn't need the app in the classes any more. this means that only this particular commit is really necessary.

it could be that the whole change of this PR could be helpful in the future but it isn't necessary at the moment!

even further thought: we could move that functionality to pywebpack. here. it wouldn't be possible to use current_app in pywebpack but we could read the variable from os.environ and that could be also set in invenio-cli or on a docker image build as environment variable.

if those reverts will be done, the reverts described here in invenio-assets have to be applied too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@utnapischtim