Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add Project CI Lint support#1896

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nejch merged 1 commit intomainfromjlvillal/ci_lint
Jul 5, 2022
Merged

feat: add Project CI Lint support#1896

nejch merged 1 commit intomainfromjlvillal/ci_lint
Jul 5, 2022

Conversation

JohnVillalovos
Copy link
Member

@JohnVillalovosJohnVillalovos commentedFeb 19, 2022
edited
Loading

Add support for validating a project's CI configuration [1]

[1]https://docs.gitlab.com/ee/api/lint.html

GabDug reacted with thumbs up emojiGabDug reacted with heart emoji
@JohnVillalovosJohnVillalovos marked this pull request as draftFebruary 19, 2022 19:46
@JohnVillalovosJohnVillalovosforce-pushed thejlvillal/ci_lint branch 2 times, most recently from06a7021 toa8457f3CompareFebruary 19, 2022 19:47
@JohnVillalovosJohnVillalovos changed the titlewip: Add Project CI Lintfeat: add Project CI Lint supportFeb 19, 2022
@codecov-commenter
Copy link

codecov-commenter commentedFeb 19, 2022
edited
Loading

Codecov Report

Merging#1896 (d53aeb4) intomain (3df404c) willincrease coverage by0.00%.
The diff coverage is100.00%.

@@           Coverage Diff           @@##             main    #1896   +/-   ##=======================================  Coverage   95.34%   95.35%           =======================================  Files          78       78             Lines        5092     5101    +9     =======================================+ Hits         4855     4864    +9  Misses        237      237
FlagCoverage Δ
cli_func_v482.43% <88.88%> (+0.01%)⬆️
py_func_v481.18% <88.88%> (+0.01%)⬆️
unit87.02% <100.00%> (+0.02%)⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

Impacted FilesCoverage Δ
gitlab/v4/objects/projects.py100.00% <100.00%> (ø)

@nejch
Copy link
Member

Huh, this is an odd situation as we have POST and GET to the same endpoint doing almost the same thing, plus the POST on the instance level (https://github.com/python-gitlab/python-gitlab/blob/main/gitlab/client.py#L374-L393).

I would expect just POST withoutcontent to default to using the latest HEAD instead of doing GET/POST, interesting. Might be a topic for the GitLab API call as well.

@JohnVillalovos
Copy link
MemberAuthor

Huh, this is an odd situation as we have POST and GET to the same endpoint doing almost the same thing, plus the POST on the instance level (https://github.com/python-gitlab/python-gitlab/blob/main/gitlab/client.py#L374-L393).

I would expect just POST withoutcontent to default to using the latest HEAD instead of doing GET/POST, interesting. Might be a topic for the GitLab API call as well.

I'm not sure my approach is the best way to do this, thus why it is in draft status. It did work though in my testing as I had a request to do this at my work.

I'll get back to it after my other PRs get merged or rejected 😁

@GabDug
Copy link

Hello! I hope this PR could be merged, thanks for the work :)

To add my two cents,

  • About the best approach considering the different API calls. I think the lib could implement them, as I don't expect client libs to try and solve "weirdnesses" in the server API.
  • Nevertheless, theGET endpoint could probably be included as a first step, as it provides value not included in the instance level call (get merged_yaml without providing any context other than the project id).
nejch and JohnVillalovos reacted with thumbs up emoji

@nejch
Copy link
Member

nejch commentedJun 14, 2022
edited
Loading

Thanks for getting in touch@GabDug!

The discussion I think was mostly about the fact that if we follow our normal Mixin patterns, we will end up with bothproject.ci_lint.get() andproject.ci_lint.create(my_yaml_content) - which might look a bit unintuitive in this specific case, but otherwise I agree.

Edit: but now that I look at it again, I guess it's not that bad.

@JohnVillalovosJohnVillalovos marked this pull request as ready for reviewJuly 4, 2022 21:05
@JohnVillalovos
Copy link
MemberAuthor

@nejch and@GabDug I finally got back to this. Now has docs and tests. So it is finally ready for review.

GabDug reacted with heart emoji

@JohnVillalovosJohnVillalovosforce-pushed thejlvillal/ci_lint branch 2 times, most recently frombf2fa02 tod53aeb4CompareJuly 4, 2022 22:28
Add support for validating a project's CI configuration [1][1]https://docs.gitlab.com/ee/api/lint.html
@nejchnejch merged commitd15fea0 intomainJul 5, 2022
@nejchnejch deleted the jlvillal/ci_lint branchJuly 5, 2022 06:31
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nejchnejchnejch 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.

4 participants
@JohnVillalovos@codecov-commenter@nejch@GabDug

[8]ページ先頭

©2009-2025 Movatter.jp