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

Use stable oasdiff docker image tag instead of main #57

Merged
merged 2 commits into from
Sep 26, 2024
Merged

Use stable oasdiff docker image tag instead of main #57

merged 2 commits into from
Sep 26, 2024

Conversation

shahakL
Copy link
Contributor

@shahakL shahakL commented Sep 24, 2024

Currently the action uses the main tag for base docker image tufin/oasdiff, however it seems the latest tag is more suitable - it is updated together with the release of latest versions, while the main tag lags behind. See https://hub.docker.com/r/tufin/oasdiff/tags

@shahakL
Copy link
Contributor Author

shahakL commented Sep 24, 2024

@effoeffi I would appreciate your opinion on this one, thanks in advance!

@effoeffi
Copy link
Contributor

@reuvenharrison can you please advice?

@reuvenharrison
Copy link

I agree latest is better than main

@shahakL
Copy link
Contributor Author

shahakL commented Sep 26, 2024

Hi! It seems the expected outputs were outdated, so I updated them to match what is expected with the latest version of oasdiff. I also removed unused params in the tests - format parameter (currently only supported by the diff action) and fail-on-diff (support for it dropped here #52)

@effoeffi
Copy link
Contributor

I agree latest is better than main

The latest tag depends on the last build, such as a branch. @reuvenharrison suggested offline to use stable instead.

effoeffi
effoeffi reviewed Sep 26, 2024
View reviewed changes
.github/workflows/test.yaml Outdated
@@ -76,6 +76,8 @@ jobs:
oasdiff_breaking:
runs-on: ubuntu-latest
name: Test breaking changes
env:
EXPECTED_OUTPUT: "1 changes: 1 error, 0 warning, 0 info"
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets add a prefix to our environment variables, like: OASDIFF_ACTION_TEST_EXPECTED_OUTPUT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@shahakL shahakL changed the title Use latest image tag instead of main Use stable image tag instead of main Sep 26, 2024
@effoeffi
Copy link
Contributor

@shahakL can you please check? https://github.com/oasdiff/oasdiff-action/actions/runs/11050605448/job/30703077487?pr=57

@shahakL
Copy link
Contributor Author

shahakL commented Sep 26, 2024

@shahakL can you please check? https://github.com/oasdiff/oasdiff-action/actions/runs/11050605448/job/30703077487?pr=57

Sorry, missed that one. It should now be fixed!

@effoeffi effoeffi changed the title Use stable image tag instead of main Use stable oasdiff docker image tag instead of main Sep 26, 2024
@effoeffi effoeffi merged commit 1c611ff into oasdiff:main Sep 26, 2024
10 checks passed
@shahakL shahakL deleted the use_latest_image branch September 26, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@effoeffi effoeffi effoeffi left review comments

@reuvenharrison reuvenharrison reuvenharrison 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.
3 participants
@shahakL @effoeffi @reuvenharrison