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

Improve client 1 to implement 1 #21

Merged
merged 4 commits into from
May 13, 2020
Merged

Improve client 1 to implement 1 #21

merged 4 commits into from
May 13, 2020

Conversation

schettn
Copy link
Member

@schettn schettn commented May 13, 2020

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change?
  • Have you tested your changes with successful results?

Type of Changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

What is the current behavior? (link to any open issues here)

What is the new behavior (if this is a feature change)?

Other information:

  • Ref: 🐍

A description for the url parameter has been added.
Now the client can handle string in "protocol://foo.bar" and "foo.bar" format. By default the http protocol will be appended if no other protocol is specified.
The GitlabClient has been replaced with a WebClient. This has been done because gitlab does not require a own client. The WebClient can handle all its needs.
@schettn schettn added bug Something isn't working enhancement New feature or request labels May 13, 2020
@schettn schettn requested review from Aichnerc and pinterid May 13, 2020 13:26
@schettn schettn self-assigned this May 13, 2020
@Aichnerc Aichnerc added this to Reviewer approved in SNEK Network for Engineers and Knowledged via automation May 13, 2020
Copy link
Member

@Aichnerc Aichnerc left a comment

Choose a reason for hiding this comment

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

Looks good to me. 🐋

ep.url = ((url: string) => {
if (!/^(?:f|ht)tps?\:\/\//.test(url)) {
url = "http://" + url;
}
Copy link
Member

Choose a reason for hiding this comment

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

Add newline.

Copy link
Member

@pinterid pinterid left a comment

Choose a reason for hiding this comment

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

Looks good after a change.

SNEK Network for Engineers and Knowledged automation moved this from Reviewer approved to Review in progress May 13, 2020
The code quality has been improved due to a request of @pinterid.
@schettn schettn requested a review from pinterid May 13, 2020 14:11
SNEK Network for Engineers and Knowledged automation moved this from Review in progress to Reviewer approved May 13, 2020
Copy link
Member

@pinterid pinterid left a comment

Choose a reason for hiding this comment

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

Nice.

@pinterid pinterid merged commit 6943d36 into implement-1 May 13, 2020
SNEK Network for Engineers and Knowledged automation moved this from Reviewer approved to Done May 13, 2020
schettn added a commit to snek-at/intel that referenced this pull request May 13, 2020
Due to a new snek-client release, the client has to be replaced.
Ref: snek-at/client#21
@schettn schettn mentioned this pull request May 29, 2020
6 tasks
@schettn schettn modified the milestones: v1, v0.1 May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

There should be a URL checker for every client There should be a WebClient instead of a GitlabClient
3 participants