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

Regened rest clients #91

Merged
merged 10 commits into from
Jan 25, 2022
Merged

Regened rest clients #91

merged 10 commits into from
Jan 25, 2022

Conversation

Alexamakans
Copy link
Member

@Alexamakans Alexamakans commented Nov 18, 2021

Summary

  • Regenerated rest clients
Repository Using Version
wharf-api v5.0.0
wharf-provider-azuredevops v2.0.1
wharf-provider-github v2.0.0
wharf-provider-gitlab v1.3.0
  • Updated code to use the new endpoints and type/function names.

Motivation

Keep up-to-date.


Closes #73

wharf-api v5.0.0-rc.1, wharf-provider-github v2.0.0, wharf-provider-gitlab v1.2.0, wharf-provider-azuredevops v2.0.1
@Alexamakans Alexamakans added the enhancement New feature or request label Nov 18, 2021
@Alexamakans Alexamakans self-assigned this Nov 18, 2021
@Alexamakans Alexamakans added this to In progress in Backlog via automation Nov 18, 2021
@Alexamakans Alexamakans marked this pull request as ready for review January 20, 2022 11:01
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.

There are so many changed files that GitHub stopped providing syntax highligting for the second half /shrug

CHANGELOG.md Outdated Show resolved Hide resolved
projects/api-client/src/model/mainBranch.ts Outdated Show resolved Hide resolved
projects/import-github-client/src/model/problemResponse.ts Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Backlog automation moved this from In progress to Review in progress Jan 20, 2022
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.

LGTM!

Regarding this discussion: #91 (comment)
I think it's better to turn that into an issue and deal with it in a different PR.

Also: the PR description is outdated.

@@ -54,7 +54,7 @@ export class ActionsModalComponent implements OnInit, OnDestroy {
if (this.actionName === this.projectUtilsService.runAllActionName) {
this.actionName = 'ALL';
}
this.projectService.projectProjectidStageRunPost(
this.projectService.oldStartProjectBuild(
Copy link
Contributor

@applejag applejag Jan 21, 2022

Choose a reason for hiding this comment

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

I was worried about breaking compatibility, but hey we mostly only do GET requests from wharf-web (which have barely changed), with the exception of this POST build request. Very nice!

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to be careful though once we start using wharf-api v5.0.0-specific query parameters or endpoints. Maybe we should (in a different issue & PR) just do a major version bump and switch to using the new endpoints, so that we don't trip on this.

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: kalle (jag) <kalle.jillheden@iver.se>
@Alexamakans
Copy link
Member Author

Regarding this discussion: #91 (comment) I think it's better to turn that into an issue and deal with it in a different PR.

Created issue for this: #105

Comment on lines +197 to +204








Copy link
Contributor

Choose a reason for hiding this comment

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

nit: intresting whitepace here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, same as before it's an artifact of the swagger code generation.
Possibly a stylistic choice by them 🤷🏻‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. They've a bunch of if-statements in their templates, such as:

{{ if foo }}
  foo!
{{ end }}
{{ if bar }}
  bar!
{{ end }}
{{ if moo }}
  moo!
{{ end }}

Which if none of foo, bar, or moo is set, then it results in 3 empty lines. One for each newline between the {{ end }} and next {{ if }} statements.

@Alexamakans Alexamakans merged commit e969bec into master Jan 25, 2022
@Alexamakans Alexamakans deleted the feature/regen-rest-clients branch January 25, 2022 08:12
@applejag applejag mentioned this pull request May 9, 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
Review in progress
Development

Successfully merging this pull request may close these issues.

Update function names to match generated names
3 participants