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

Upgrade codemirror and lezer to the latest version #10841

Merged
merged 3 commits into from Jun 13, 2022

Conversation

Nexucis
Copy link
Member

@Nexucis Nexucis commented Jun 9, 2022

No description provided.

@Nexucis
Copy link
Member Author

Nexucis commented Jun 9, 2022

build is failing because of the warnings :/

@roidelapluie
Copy link
Member

Do you think so? 243 seems to indicate a permission denied issue, which is caused by trying to crate /.npm

@Nexucis
Copy link
Member Author

Nexucis commented Jun 9, 2022

ah I was looking at the tests jobs where the warning was the root cause.

But yeah the build job is still failing which is weird. I mean concerning the UI, it is doing the same thing than the job ui_test.

@Nexucis
Copy link
Member Author

Nexucis commented Jun 9, 2022

How do you know it is trying to create a folder .npm @roidelapluie ? And why does it work in other jobs ?

@MrDeerly
Copy link

MrDeerly commented Jun 9, 2022

Looks good for me. Just make sure to bump lezer-promql in codemirror-promql (not sure if that's done in an automated way)

@Nexucis
Copy link
Member Author

Nexucis commented Jun 9, 2022

yeah it's done automatically @MrDeerly. It's a npm workspace, so codemirror-promql is currently using the local version of lezer-promql.

@Nexucis
Copy link
Member Author

Nexucis commented Jun 9, 2022

By looking at #10840, it doesn't look like the build failure is related to this PR actually. It looks like it's a general issue actually :/

@@ -45,7 +45,7 @@ ui-install:

.PHONY: ui-build
ui-build:
cd $(UI_PATH) && npm run build
cd $(UI_PATH) && CI="" npm run build
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason behind this unsetting of the CI variable?

Copy link
Member Author

@Nexucis Nexucis Jun 9, 2022

Choose a reason for hiding this comment

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

it's because there are some warnings raised coming from some dependencies in node-modules. I didn't find other way to ignore them than like that.
Like that warnings are not treated as errors

@Nexucis Nexucis force-pushed the nexucis/codemirror-upgrade branch from 505839c to c5e148d Compare June 9, 2022 13:13
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
@Nexucis Nexucis force-pushed the nexucis/codemirror-upgrade branch from c5e148d to 30fd158 Compare June 13, 2022 15:08
@juliusv
Copy link
Member

juliusv commented Jun 13, 2022

👍 Don't super like the CI="" part, but it seems like everyone is just doing that :/

@juliusv juliusv merged commit 5d1756c into main Jun 13, 2022
@juliusv juliusv deleted the nexucis/codemirror-upgrade branch June 13, 2022 15:55
roidelapluie pushed a commit to roidelapluie/prometheus that referenced this pull request Jun 22, 2022
* bump codemirror to v0.20.x and lezer to v.0.16.x

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>

* bump codemirror to v6 and lezer to v1

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>

* stop treating warning as error for UI

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
@MrDeerly
Copy link

MrDeerly commented Jul 14, 2022

@Nexucis with this change the lezer-promql isn't installed in codemirror-promql anymore as a dependency. Probable codemirror-promql was published and build before lezer-promql?

Contents of /node_modules/@prometheus-io/codemirror-promql/node_modules:

lru-cache

^ #11017

@Nexucis
Copy link
Member Author

Nexucis commented Jul 14, 2022

@MrDeerly it should be installed in @promethus-io folder, since it is now published as @prometheus-io/lezer-promql

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.

None yet

4 participants