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.

Sign up for GitHub

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

Jump to bottom

Remove check_version in common_functions.sh #4954

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Remove check_version in common_functions.sh #4954

merged 1 commit into from
Feb 6, 2024

Conversation

annaibm
Copy link
Contributor

@annaibm annaibm commented Jan 10, 2024 •
edited
Loading

resolves : #4949

@annaibm annaibm force-pushed the remove_checkversion_4949 branch from a635f5f to e4a9277 Compare January 10, 2024 21:54
@annaibm
Copy link
Contributor Author

annaibm commented Jan 11, 2024

Grinder test:
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/37316/

@annaibm annaibm marked this pull request as ready for review January 11, 2024 18:14
@llxia
Copy link
Contributor

llxia commented Jan 11, 2024 •
edited
Loading

I think https://github.com/adoptium/aqa-tests/pull/4954/files#diff-e1cbec3d1700bf35e3daef146d4667564133f0a528b700594195c7a0184e7ff5L17 should also be removed. Please check its references.

karianna
karianna requested changes Jan 11, 2024
View reviewed changes
external/build_image.sh Show resolved Hide resolved
external/common_functions.sh Outdated Show resolved Hide resolved
@annaibm annaibm force-pushed the remove_checkversion_4949 branch from e4a9277 to 52c9eb6 Compare January 11, 2024 18:53
@llxia
Copy link
Contributor

llxia commented Jan 12, 2024 •
edited
Loading

@annaibm did you test the latest change in Grinder? supported_versions seems to be referenced in other places: https://github.com/search?q=repo%3Aadoptium%2Faqa-tests%20supported_versions&type=code

@annaibm
Copy link
Contributor Author

annaibm commented Jan 12, 2024 •
edited
Loading

@llxia I have tested in Grinders :
1.37316,
2.37315.
Other than the above, Is there any other specific tests for external that is crucial to be done?
Should I keep https://github.com/adoptium/aqa-tests/pull/4954/files#diff-e1cbec3d1700bf35e3daef146d4667564133f0a528b700594195c7a0184e7ff5L17 , if its used in other places. Not sure in Grinder it was not breaking/failing ?

@annaibm annaibm marked this pull request as draft January 12, 2024 14:36
@annaibm annaibm force-pushed the remove_checkversion_4949 branch 2 times, most recently from 5a29c49 to fb46146 Compare January 17, 2024 15:09
@annaibm annaibm force-pushed the remove_checkversion_4949 branch 3 times, most recently from bd9bd9e to 3e1618e Compare January 30, 2024 18:02
@llxia
Copy link
Contributor

llxia commented Jan 30, 2024

The suggestion was to remove supported_versions. However, supported_versions is still in the PR. Please ensure the Grinder is testing the correct change.

@annaibm annaibm force-pushed the remove_checkversion_4949 branch 3 times, most recently from 250c729 to 4c09020 Compare January 30, 2024 22:25
@annaibm
Copy link
Contributor Author

annaibm commented Jan 31, 2024

Grinder tests:
37729 - aot_test
37733 - criu-functional

@annaibm annaibm force-pushed the remove_checkversion_4949 branch from 4c09020 to ac9a0ca Compare January 31, 2024 15:44
@annaibm
Copy link
Contributor Author

annaibm commented Jan 31, 2024

Grinder test:
dev.external -> 37738

LongyuZhang
LongyuZhang reviewed Jan 31, 2024
View reviewed changes
external/build_all.sh Outdated Show resolved Hide resolved
@annaibm
Remove check_version in common_functions.sh
3593e29 
- remove check_version function
- remove set_version function
- remove supported_versions
resolves : adoptium#4949

Signed-off-by: Anna Babu Palathingal <[email protected]>
@annaibm annaibm force-pushed the remove_checkversion_4949 branch from ac9a0ca to 3593e29 Compare January 31, 2024 19:16
@annaibm annaibm requested a review from LongyuZhang January 31, 2024 19:26
LongyuZhang
LongyuZhang approved these changes Jan 31, 2024
View reviewed changes
Copy link
Contributor

@LongyuZhang LongyuZhang left a comment

Choose a reason for hiding this comment

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

LGTM

@LongyuZhang
Copy link
Contributor

LongyuZhang commented Jan 31, 2024 •
edited
Loading

Grinder test: dev.external -> 37738

The failure is not related to this PR, will fix in other PRs. Same for criu-functional.

@LongyuZhang LongyuZhang marked this pull request as ready for review February 6, 2024 14:31
@LongyuZhang LongyuZhang merged commit e32dc86 into adoptium:master Feb 6, 2024
3 checks passed
@annaibm annaibm deleted the remove_checkversion_4949 branch March 12, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LongyuZhang LongyuZhang LongyuZhang approved these changes

@karianna karianna karianna approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove check_version() in common_functions.sh for external tests
4 participants
@annaibm @llxia @LongyuZhang @karianna