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

chore(updatecli): Adds documentation to the script and enhances the jq command. #914

Merged
merged 5 commits into from
Dec 7, 2024

Conversation

gounthar
Copy link
Contributor

@gounthar gounthar commented Dec 2, 2024

This is a follow-up PR to #907.

I refined the tag selection logic in jq by implementing a filter for tags containing hyphens.
To enhance code understanding, I added explanatory comments that provide additional context about the data retrieved from the RedHat REST API.

A comprehensive markdown documentation detailing the API's functionality is currently in development and will be included in an upcoming pull request.

Testing done

./updatecli/scripts/ubi9-latest-tag.sh
9.5-1732804088

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@gounthar gounthar requested a review from a team as a code owner December 2, 2024 14:24
@dduportal dduportal changed the title chore(updatecli): Adds documentation to the script and enhances the jq command. docs(updatecli): Adds documentation to the script and enhances the jq command. Dec 2, 2024
@dduportal dduportal changed the title docs(updatecli): Adds documentation to the script and enhances the jq command. chore(updatecli): Adds documentation to the script and enhances the jq command. Dec 2, 2024
@dduportal dduportal added the chore label Dec 2, 2024
dduportal
dduportal previously approved these changes Dec 2, 2024
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

Checks says:

✔ Bump UBI9 version:
	Source:
		✔ [latestVersion] Get the latest UBI9 Linux version
	Condition:
		✔ [checkUbi9DockerImage] Check if the container image "ubi9" is available
	Target:
		✔ [updateDockerBake] Update the default value of the variable UBI9_TAG in the docker-bake.hcl
		✔ [updateDockerfile] Update the value of the base image (ARG UBI9_TAG) in the Dockerfile

That looks good to me!

Let's have a secondary review by @lemeurherve as he asked for some of these changes in previous PRs

@dduportal dduportal requested a review from lemeurherve December 2, 2024 14:30
Copy link
Contributor

@lemeurherveCB lemeurherveCB 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 the follow-up!

Some suggestions below.

updatecli/scripts/ubi9-latest-tag.sh Outdated Show resolved Hide resolved
updatecli/scripts/ubi9-latest-tag.sh Outdated Show resolved Hide resolved
updatecli/scripts/ubi9-latest-tag.sh Outdated Show resolved Hide resolved
updatecli/scripts/ubi9-latest-tag.sh Outdated Show resolved Hide resolved
updatecli/scripts/ubi9-latest-tag.sh Outdated Show resolved Hide resolved
Thanks for the review!

Co-authored-by: lemeurherveCB <[email protected]>
@lemeurherveCB
Copy link
Contributor

lemeurherveCB commented Dec 3, 2024

Reposting a comment from an automatically hidden thread:

A comprehensive markdown documentation detailing the API's functionality is currently in development and will be included in an upcoming pull request.

remark(not concerning this PR): a doc for RedHat API? If so, I don't think it has a place in this repo as it's not a core part of Jenkins agents, I would suggest not spending time on this.

Copy link
Contributor

@lemeurherveCB lemeurherveCB left a comment

Choose a reason for hiding this comment

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

LGTM, thx!

Co-authored-by: lemeurherveCB <[email protected]>
@dduportal dduportal enabled auto-merge December 7, 2024 15:33
@dduportal dduportal added chore and removed chore labels Dec 7, 2024
@dduportal dduportal merged commit c725374 into jenkinsci:master Dec 7, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants