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

Made wharf-api v5.0.0 compatible #43

Merged
merged 25 commits into from
Feb 24, 2022

Conversation

Alexamakans
Copy link
Member

@Alexamakans Alexamakans commented Jan 18, 2022

Summary

Updated to use wharf-api-client-go v2.0.0, making it wharf-api v5.0.0 compatible, but dropping support for wharf-api v4.2.0 and below.

The testing code has not been updated and is therefore not functional currently. See Unresolved questions.

Motivation

Keep up-to-date.

Resolved questions

  • The testing code has not been updated and is therefore not functional currently.
    If we want to keep it the way it is (updated so it works) then I can spend time updating it.
    Otherwise I will remove it, and we can come back to it when we have decided on how we want to write our tests.
    Should we spend time updating it, or remove it for now?
    • Updated, and some added for refreshing a project.

Closes #6, closes #42

@Alexamakans Alexamakans added the enhancement New feature or request label Jan 18, 2022
@Alexamakans Alexamakans self-assigned this Jan 18, 2022
@Alexamakans Alexamakans added this to In progress in Backlog via automation Jan 18, 2022
@Alexamakans Alexamakans marked this pull request as draft January 18, 2022 13:41
@Alexamakans
Copy link
Member Author

Alexamakans commented Jan 18, 2022

Currently runnable with go run ., Docker and GitHub builds fail because tests don't work.

CHANGELOG.md Outdated Show resolved Hide resolved
iwharfclientfetcher.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fredx30 fredx30 left a comment

Choose a reason for hiding this comment

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

Seems to me as if this could be redrafted untill the testing issue i resolved.

Copy link
Contributor

@applejag applejag left a comment

Choose a reason for hiding this comment

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

Marking as "request changes" again to get it out of our PR tracker as still expecting changes

@Alexamakans
Copy link
Member Author

Fixed tests.

No coverage increase. Due to changes in the code, coverage % has decreased. There should be no change to the paths that are tested though.

Copy link
Contributor

@applejag applejag left a comment

Choose a reason for hiding this comment

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

Lunchbreak between reviews, will continue to review later

iwharfclientfetcher.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
import.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fredx30 fredx30 left a comment

Choose a reason for hiding this comment

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

A bit hard to review- not entierly sure why or how some of these changes came about but if its tested im sure it will be fine.

import.go Show resolved Hide resolved
@Alexamakans
Copy link
Member Author

Mainly projects with spaces in their names broke, but I think it's also possible for the actual path to be different from the display name.

Added a fallback to search for the project by name and comparing groupName with the namespaces' different name variants.
This takes a fairly long time, but it does the trick.

This will also affect refreshing those projects, until logic for remote project IDs is implemented anyways.

gitlab.go Outdated Show resolved Hide resolved
gitlab.go Outdated Show resolved Hide resolved
Co-authored-by: kalle (jag) <kalle.fagerberg@iver.se>
@Alexamakans Alexamakans merged commit a21e50a into master Feb 24, 2022
@Alexamakans Alexamakans deleted the feature/make-wharf-api-v5.0.0-compatible branch February 24, 2022 16:02
@applejag applejag mentioned this pull request May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Backlog
In progress
3 participants