Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 5, 2022
Merged

feat: add Project CI Lint support #1896

merged 1 commit into from
Jul 5, 2022

Conversation

JohnVillalovos
Copy link
Member

@JohnVillalovos JohnVillalovos commented Feb 19, 2022

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

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

@JohnVillalovos JohnVillalovos marked this pull request as draft February 19, 2022 19:46
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/ci_lint branch 2 times, most recently from 06a7021 to a8457f3 Compare February 19, 2022 19:47
@JohnVillalovos JohnVillalovos changed the title wip: Add Project CI Lint feat: add Project CI Lint support Feb 19, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2022

Codecov Report

Merging #1896 (d53aeb4) into main (3df404c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1896   +/-   ##
=======================================
  Coverage   95.34%   95.35%           
=======================================
  Files          78       78           
  Lines        5092     5101    +9     
=======================================
+ Hits         4855     4864    +9     
  Misses        237      237           
Flag Coverage Δ
cli_func_v4 82.43% <88.88%> (+0.01%) ⬆️
py_func_v4 81.18% <88.88%> (+0.01%) ⬆️
unit 87.02% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
gitlab/v4/objects/projects.py 100.00% <100.00%> (ø)

@nejch
Copy link
Member

nejch commented Mar 10, 2022

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 without content 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
Member Author

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 without content 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

GabDug commented Jun 14, 2022

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, the GET 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
Copy link
Member

nejch commented Jun 14, 2022

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 both project.ci_lint.get() and project.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.

@JohnVillalovos JohnVillalovos marked this pull request as ready for review July 4, 2022 21:05
@JohnVillalovos
Copy link
Member Author

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

@JohnVillalovos JohnVillalovos force-pushed the jlvillal/ci_lint branch 2 times, most recently from bf2fa02 to d53aeb4 Compare July 4, 2022 22:28
@JohnVillalovos JohnVillalovos requested a review from nejch July 4, 2022 22:29
Add support for validating a project's CI configuration [1]

[1] https://docs.gitlab.com/ee/api/lint.html
@nejch nejch merged commit d15fea0 into main Jul 5, 2022
@nejch nejch deleted the jlvillal/ci_lint branch July 5, 2022 06:31
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.

4 participants