Skip to content

JS-652 Changing the dependency of meta and generated-meta #5217

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

Merged
merged 5 commits into from
Mar 20, 2025
Merged

Conversation

zglicz
Copy link
Contributor

@zglicz zglicz commented Mar 19, 2025

JS-652

As this was a bit out of order, we should not have hand written code depend on generated code. This way, we can access meta.js before the generated-meta.js is presented.

With these changes, I was able to remove the dependency of javascript-checks on bridge. This way, the bridge starts building instantly when using the parallel option of maven build!

@zglicz zglicz requested a review from a team March 19, 2025 15:53
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title Changing the dependency of meta and generated-meta JS-652 Changing the dependency of meta and generated-meta Mar 19, 2025
Copy link
Contributor Author

@zglicz zglicz left a comment

Choose a reason for hiding this comment

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

-500 lines, still valuable :)

Copy link
Contributor

@vdiez vdiez 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 to me. some points:

  • for me npm ci is a prerequisite. we should have dependencies installed:
    • no need to run npx in scripts
    • we can always prettify
  • seems changes on the git submodules have been pushed. can you run git submodules update locally? I don't think there have been changes on them lately so so may have an old commit locally. In any case, don't add them to this PR pls

Copy link
Contributor

@ericmorand-sonarsource ericmorand-sonarsource left a comment

Choose a reason for hiding this comment

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

Thats' great, thank you. We will all benefit from the reduced build duration.

@zglicz zglicz enabled auto-merge (squash) March 20, 2025 13:45
Copy link

@zglicz zglicz merged commit f3a860e into master Mar 20, 2025
18 checks passed
@zglicz zglicz deleted the dependendance branch March 20, 2025 13:55
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