-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
change distribution_release to 15.6 #3798
base: master
Are you sure you want to change the base?
Conversation
6f58d5f
to
4f497a6
Compare
@AswathySK You'll need to sign the Eclipse CLA. Vagrant failure is unrelated |
@karianna , |
Question for @sxa |
Signed-off-by: Aswathy S Kumar <aswathyskumar144@gmail.com>
4f497a6
to
0ff508e
Compare
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.
@karianna , Why cant we make it dynamic using {{ ansible_distribution_release }} instead?
What do you think @Haroon-Khel - looks like you changed the URL path from SLE_15_SP3
to the current format in https://github.com/adoptium/infrastructure/pull/3060/files - if we can use an automatically set distribution version variable instead that sounds reasonable to me.
(Also tagging @steelhead31 since he's looked at some SLES stuff too)
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 dont have a problem with it being dynamic, saves us having to keep changing the hardcoded variable
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.
This pr is fine to merge as it is btw
@Haroon-Khel Noting also that we have a 15.4 reference in Line 6 in 822366e
|
@sxa @karianna and @Haroon-Khel Can you please take a look at this issue as well: #3799 |
There is one more issue @karianna , @Haroon-Khel , @sxa , While trying to add the Devel-Tools repo with the
As a fix tried adding the autorefresh parameter to the
Succesful execution of this task was able to be achieved by making the following change:
|
Signed-off-by: Aswathy S Kumar <aswathyskumar144@gmail.com>
@@ -23,7 +23,8 @@ | |||
|
|||
- name: Add Devel-Tools repository (SLES15) | |||
zypper_repository: | |||
repo: https://download.opensuse.org/repositories/devel:/tools/15.5/devel:tools.repo | |||
name: devel-tools | |||
repo: 'https://download.opensuse.org/repositories/devel:/tools/15.{{ ansible_distribution_release }}/' |
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.
Pedantry, would it make more sense to make ansible_distribution_release the entire 15.6? Later on it might move to 16.x....
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.
@karianna , I did not change it because the when condition checks whether it is sles 15 or not.
We can either make it {{ ansible_distribution_major_version }}.{{ ansible_distribution_release }} or remove the when condition :
when:
- ansible_distribution_major_version == "15"
Need to change the distribution release version from 15.5 to 15.6 in the common role to add the devel-tools repo.
Can we make a change to make this dynamic?
Instead of using the hardcoded version , cant we make it dynamic?
For example instead of
15.5
cant we make it{{ ansible_distribution_major_version }}.{{ ansible_distribution_release }}
Checklist