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

Fix LSP test using the New Gradle Wizard #7267

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

lkishalmi
Copy link
Contributor

Trivial change, as far as I could test it seems to work.

@lkishalmi lkishalmi added the LSP [ci] enable Language Server Protocol tests label Apr 14, 2024
@lkishalmi lkishalmi added this to the NB22 milestone Apr 14, 2024
@lkishalmi lkishalmi requested review from mbien and sdedic April 14, 2024 16:50
@lkishalmi lkishalmi marked this pull request as ready for review April 14, 2024 16:50
@mbien
Copy link
Member

mbien commented Apr 14, 2024

@lkishalmi the ProjectViewTest is using gradle project support in the test cases, do you think I should change CI so that LSP tests run when the Gradle label is set?

Copy link
Member

@mbien mbien 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 looking into this!

@mbien mbien merged commit 662cc57 into apache:master Apr 14, 2024
34 checks passed
@neilcsmith-net
Copy link
Member

@mbien other option might be to relook at using merge queue?

@mbien
Copy link
Member

mbien commented Apr 14, 2024

@neilcsmith-net I don't quite see how merge queues relate to this here. Merge queues also require reliable tests which NB is far from at the moment. Failing tests in a merge queue will trigger automated response - at least thats how the classic mechanism works.

@neilcsmith-net
Copy link
Member

@mbien my understanding (maybe incorrect) would be that it could allow for all tests to run before the merge to master occurs, so could act as a failsafe for missing labels rather than adding more tests on labels where they might not be necessary. Just a thought. Can follow up on that elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants