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

test: update tests to depends on platform go middleware #11

Conversation

avisiedo
Copy link
Contributor

@avisiedo avisiedo commented Jan 5, 2024

This change update the tests to fit the changes at #2

It uses the pointy library, which after the refactor of the tests the helpers at util.go are not required and removed in a second commit.

Depends on: #2

This change exchange the dependency on:

"github.com/redhatinsights/module-update-router/identity"

by:

"github.com/redhatinsights/platform-go-middlewares/identity"

to avoid transition dependency on the dead repository:

https://github.com/mitchellh/osext

Signed-off-by: Alejandro Visiedo <avisiedo@redhat.com>
Signed-off-by: Alejandro Visiedo <avisiedo@redhat.com>
This change update the tests to fit the changes (now depending on
Identity from platform-go-middlewares). Additionally, it is used pointy
methods to generate pointer to values.

Signed-off-by: Alejandro Visiedo <avisiedo@redhat.com>
Remove the utils.go file as with the current state they are not needed;
and this will fix the linter violation in the pipeline.

Signed-off-by: Alejandro Visiedo <avisiedo@redhat.com>
@avisiedo
Copy link
Contributor Author

@subpop Continuing here from comments at #2

For clarification, as I see three different reads, which option is preferred?

(1) Do not squash any commit and merge the PR as it is.
(2) Squahs 262ed7d and 6cf4d46 in one commit (fix) and squash b003581 and acc9004 in a second commit (test), and merge PR with that two new commits.
(3) Squash everything in one unique commit (fix: ....) and merge the PR.

In any case, close #2 as that changes are included here.

What is the approached you prefer?

Copy link
Owner

@subpop subpop left a comment

Choose a reason for hiding this comment

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

This PR, as is is fine. These commits are well organized and logical enough. For the sake of simplicity, I'll just accept and merge this PR as is and close #2. Thank you for bearing with me!

@subpop subpop merged commit daeb33d into subpop:main Jan 18, 2024
4 checks passed
@avisiedo
Copy link
Contributor Author

thanks for your time! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants