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

pin dependencies to versions for go 1.21 and remove update flag #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jdubrick
Copy link
Contributor

Description of Changes

Summarize the changes you made as part of this PR.

This PR removes the -u flag from our go get statements and also changes the pins from sha to a version to make it easier to identify what we are running on. Both of these versions have go.mod files that only contain dependencies for go 1.21 or earlier so we won't encounter the issue of requiring go 1.22+ anymore.

Since we are not always going to be on the latest Go version (historically have been lagging behind by a version or two) it is best to remove the -u flag to ensure we don't encounter this issue again where some dependency outpaces our Go version.

Related Issue(s)

Link the GitHub/GitLab/JIRA issues that are related to this PR.

resolves devfile/api#1643

Acceptance Criteria

Tests

  • Test Coverage
    • Are your changes sufficiently tested, and are any applicable test cases added or updated to cover your changes?
  • Gosec scans

Documentation

  • Does the registry operator documentation need to be updated with your changes?

Tests Performed

Explain what tests you personally ran to ensure the changes are functioning as expected.

How To Test

Instructions for the reviewer on how to test your changes.

Running Unit Tests

Running Integration Tests

Notes To Reviewer

Any notes you would like to include for the reviewer.

Copy link
Contributor

@thepetk thepetk left a comment

Choose a reason for hiding this comment

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

lgtm as long as all status checks are passing (this was the issue in the first place).

It also unblocks #100

Copy link

openshift-ci bot commented Oct 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jdubrick, thepetk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Jdubrick
Copy link
Contributor Author

lgtm as long as all status checks are passing (this was the issue in the first place).

It also unblocks #100

Not entirely sure why the build fails since nothing related to it was changed @thepetk

@thepetk
Copy link
Contributor

thepetk commented Oct 18, 2024

Hmm.. the nil pointer issue is the same issue we were discussing in the past right? I'm refering to the operator container image check failure.

@thepetk
Copy link
Contributor

thepetk commented Oct 18, 2024

lgtm as long as all status checks are passing (this was the issue in the first place).
It also unblocks #100

Not entirely sure why the build fails since nothing related to it was changed @thepetk

you beat me to the comment :D

+1 is not related with the updates. It can be confirmed locally too.

Yeah I think a quick investigation on this might be helpful (if we haven't done it yet!), I remember (IIRC ofc) we have seen it some times occuring.

@Jdubrick
Copy link
Contributor Author

lgtm as long as all status checks are passing (this was the issue in the first place).
It also unblocks #100

Not entirely sure why the build fails since nothing related to it was changed @thepetk

you beat me to the comment :D

Yeah I think a quick investigation on this might be helpful (if we haven't done it yet!), I remember (IIRC ofc) we have seen it some times occuring.

Yeah it happened when I tested locally by opening a PR to my own main branch to run the actions but once I reran it everything was fine. Might just have to keep re-running the test and open an investigation issue

@thepetk
Copy link
Contributor

thepetk commented Oct 18, 2024

lgtm as long as all status checks are passing (this was the issue in the first place).
It also unblocks #100

Not entirely sure why the build fails since nothing related to it was changed @thepetk

you beat me to the comment :D
Yeah I think a quick investigation on this might be helpful (if we haven't done it yet!), I remember (IIRC ofc) we have seen it some times occuring.

Yeah it happened when I tested locally by opening a PR to my own main branch to run the actions but once I reran it everything was fine. Might just have to keep re-running the test and open an investigation issue

it feels to me github-runner related. Let's rerun tests and see.

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.

Registry operator CI check format task is failing
2 participants