-
Notifications
You must be signed in to change notification settings - Fork 26
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
Parameterization for Running the LTI Build Fixes #783 #833
Parameterization for Running the LTI Build Fixes #783 #833
Conversation
Parameterize Issue
- name: 'Checkout lsp4ij' | ||
if: env.USE_LOCAL_PLUGIN == 'true' | ||
uses: actions/checkout@v3 | ||
with: | ||
repository: redhat-developer/lsp4ij | ||
path: lsp4ij | ||
ref: main | ||
- name: 'Build Lsp4ij' | ||
if: env.USE_LOCAL_PLUGIN == 'true' | ||
working-directory: ./lsp4ij | ||
run: bash ./gradlew buildPlugin | ||
- name: 'Unzip lsp4ij file' | ||
if: env.USE_LOCAL_PLUGIN == 'true' | ||
working-directory: ./lsp4ij/build/distributions | ||
run: | | ||
unzip -o '*.zip' -d . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I might be getting confused by the number of prototypes we've had for these workflows but I thought our continuous integration builds would support building from PRs as well as the current main
branch. I'm not seeing that logic here. Do we have this somewhere else (on another branch or PR)? Can it all be integrated into the same workflow and controlled by a build parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I might be getting confused by the number of prototypes we've had for these workflows but I thought our continuous integration builds would support building from PRs as well as the current
main
branch. I'm not seeing that logic here. Do we have this somewhere else (on another branch or PR)? Can it all be integrated into the same workflow and controlled by a build parameter?
This ticket was primarily for determining the parameterization required to run the LTI builds. For easy prototyping, I tested with checking out the main branch only. There is a separate ticket for running the LTI tests against each open lsp4ij PR.
plugins = ['java', 'maven', 'gradle-java', 'properties', 'terminal', 'org.jetbrains.idea.maven', 'com.intellij.gradle', 'com.redhat.devtools.lsp4ij:' + lsp4ijVersion + '@nightly'] | ||
def lsp4ij = project.hasProperty('useLocal') && project.property('useLocal') == 'true' ? | ||
file("../lsp4ij/build/distributions/LSP4IJ/") : | ||
'com.redhat.devtools.lsp4ij:' + lsp4ijVersion + '@nightly' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you didn't change this in this PR, but noticed this is hard-coded here to pull the LSP4iJ version from the Nightly
channel. When we release we definitely want to be depending on a Stable
release of LSP4iJ for our build and testing. I wonder if we could separate that in build.gradle
like the lsp4ijVersion
variable that's being read here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you didn't change this in this PR, but noticed this is hard-coded here to pull the LSP4iJ version from the
Nightly
channel. When we release we definitely want to be depending on aStable
release of LSP4iJ for our build and testing. I wonder if we could separate that inbuild.gradle
like thelsp4ijVersion
variable that's being read here.
Since we have not yet decided which version of Lsp4ij to use for our release, we are currently specifying the nightly version. Once we decide to proceed with the stable release, we can remove the @nightly
tag and specify the stable version. For now, we are using @nightly
to test our functionalities. When we move forward with our release, we will switch to the stable version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anusreelakshmi934 I think @mrglavas 's suggestion is to parameterize the build channel the same way the lsp4ijVersion
is parameterized - i.e. the values would be @nightly
or @stable
. That way we can just update the variable at the top of build.gradle
when switching from nightly to stable or vice versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TrevCraw That is exactly what I was suggesting. @anusreelakshmi934 I hope that's clearer now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Got it. I have updated the lsp4ijVersion
variable at the top of build.gradle by appending @nightly
along with the version number. If we are to use the stable version we can remove that @nightly
and just specify the version alone.
Added all these changes to a Umbrella PR - #840. Hence closing this PR |
Fixes #783 - Parameterize Issue