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.

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

NPM: Version packages using Lerna or Nx #294

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

NPM: Version packages using Lerna or Nx #294

wants to merge 12 commits into from

Conversation

jackw
Copy link

@jackw jackw commented Mar 28, 2024

Requirements

This is WIP right now whilst I get the grafana/grafana branch together and do all the tests.

The main branch of grafana-build should be compatible with all active versions of Grafana and Grafana-Enterprise.

  • I have tested this against main in Grafana.
  • I have tested this against main in Grafana Enterprise.
  • I have tested this against all active version branches of Grafana (v10.0.x, v10.1.x, v10.2.x, etc).
  • I have tested this against all active version branches of Grafana Enterprise (v10.0.x, v10.1.x, v10.2.x, etc).

Why?

See this issue. TLDR; once these PRs are all merged we have the ability to create versioning strategies for different packages fixing the age old problem of "how do I add an NPM package to Grafana that doesn't get versioned and released with Grafana releases".

Associated PRs:

@jackw jackw self-assigned this Mar 28, 2024
@jackw jackw requested review from a team as code owners March 28, 2024 17:48
@jackw jackw changed the title [WIP] NPM: Introduce logic to version packages using Lerna or Nx [WIP] NPM: Version packages using Lerna or Nx Mar 29, 2024
@jackw jackw requested review from joshhunt and Clarity-89 March 29, 2024 14:50
daggerutil/fileexists.go Outdated Show resolved Hide resolved
@jackw
Copy link
Author

jackw commented May 13, 2024

@kminehart @aangelisc could I get some assistance with the fileexists utility and the █ [0.00s] ERROR file it spits out? Slack conversation here for context. I'm quite keen to pick this work back up and get it merged but I don't have the dagger know how to clean that up. 🙏

@kminehart
Copy link
Collaborator

kminehart commented May 16, 2024

After getting permission from @jackw I updated his branch here to change the approach. The biggest thing I changed was avoiding places where we have to execute the DAG in order to continue, which is basically any function that requires a ctx.

While it's a bit ugly to do these types of conditionals in bash, it has a number of advantages, almost all around caching. Now, if nothing has changed on the frontend, the results will stay the same. Plus, the dagger server won't complain about us trying to get a file that doesn't exist 😂

Copy link
Contributor

@aangelisc aangelisc left a comment

Choose a reason for hiding this comment

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

LGTM, it's still readable and straightforward to understand

Copy link
Author

@jackw jackw left a comment

Choose a reason for hiding this comment

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

Thanks for helping me out here @kminehart. I've left a comment related to the bash logic as I'm not sure these changes are gonna work with the associated Grafana branch.

frontend/npm.go Outdated Show resolved Hide resolved
@jackw jackw changed the title [WIP] NPM: Version packages using Lerna or Nx NPM: Version packages using Lerna or Nx May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants