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 running LSP4iJ continuous integration builds with existing LTI releases #864

Conversation

vaisakhkannan
Copy link
Contributor

@vaisakhkannan vaisakhkannan commented Jul 9, 2024

Fixes #815

A part of the fix for - #868
Implementation of running LSP4iJ continuous integration builds with existing LTI releases

Currently, the fix is running against the branch lsp4ij-market-0.0.2-integration. We can add the main and other existing LTI release tags, but those will fail because of the microshed:lsp4ij dependency. Once we have an LTI release that uses the redhat:lsp4ij plugin, we will be able to easily add the version in the tag specified in the code.

@vaisakhkannan
Copy link
Contributor Author

Copy link
Contributor

@anusreelakshmi934 anusreelakshmi934 left a comment

Choose a reason for hiding this comment

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

@vaisakhkannan can you attach the link for the latest build and cron job after the changes made?

@vaisakhkannan
Copy link
Contributor Author

vaisakhkannan commented Jul 16, 2024

@anusreelakshmi934 , Link for the build --> https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/9934992707

NOTE: With this latest change (based on the Kochi team's suggestion) Cron Job build will fail on fork cron job builds because lsp4ij-market-0.0.2-integration branch is not available in fork repo, because I removed the SpecifyRepo parameter, which is initially used to specify the repository for the step named 'Checkout liberty-tools-intellij'. Since we are running on the main branch, it will default to OpenLiberty/liberty-tools-intellij. For testing purposes on a forked repository, the fork repository owner must Specify the repository as OpenLiberty/liberty-tools-intellij in build.yaml file as shown below. There won't be no issues with this code running on reomote main branch.

repository: OpenLiberty/Liberty-tools-intellij

Link For cron job when I added repository as OpenLiberty/liberty-tools-intellij --> https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/9934517308

Link For cron job when I don't specify repository as OpenLiberty/liberty-tools-intellij --> https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/9934471195

@anusreelakshmi934
Copy link
Contributor

Link For cron job when I added repository as OpenLiberty/liberty-tools-intellij --> https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/9934517308

Shouldn't the job also run against LTI main?

@vaisakhkannan
Copy link
Contributor Author

Link For cron job when I added repository as OpenLiberty/liberty-tools-intellij --> https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/9934517308

Shouldn't the job also run against LTI main?

@anusreelakshmi934 , If we need to run on the main branch as well, we can add 'main' in the matrix tag.
like this
tag: [ 'lsp4ij-market-0.0.2-integration', 'main' ]

aparnamichael
aparnamichael previously approved these changes Jul 23, 2024
Copy link
Contributor

@aparnamichael aparnamichael 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!!

@anusreelakshmi934
Copy link
Contributor

Can you share the cronjob link here @vaisakhkannan ?

@vaisakhkannan
Copy link
Contributor Author

vaisakhkannan commented Jul 24, 2024

Can you share the cronjob link here @vaisakhkannan ?

@anusreelakshmi934 ,
https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/10072224523
https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/10045591045

Copy link
Contributor

@anusreelakshmi934 anusreelakshmi934 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

Copy link
Contributor

@mrglavas mrglavas left a comment

Choose a reason for hiding this comment

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

Approving based on seeing this in action through demos and resolution of discussions with the team that I took part in. Great work!

@vaisakhkannan vaisakhkannan merged commit 1bb8d42 into OpenLiberty:lsp4ij-market-0.0.2-integration Aug 9, 2024
2 of 3 checks passed
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.

5 participants