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

refactor: migrate schedules_processor to use python-gitlab #706

Merged
merged 19 commits into from Apr 19, 2024

Conversation

TimKnight-Opencast
Copy link
Contributor

@TimKnight-Opencast TimKnight-Opencast commented Mar 13, 2024

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.59%. Comparing base (f7c5922) to head (fc4e40a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #706      +/-   ##
==========================================
+ Coverage   85.38%   85.59%   +0.21%     
==========================================
  Files          70       69       -1     
  Lines        2784     2770      -14     
==========================================
- Hits         2377     2371       -6     
+ Misses        407      399       -8     
Files Coverage Δ
gitlabform/gitlab/__init__.py 100.00% <ø> (ø)
...tlabform/processors/project/schedules_processor.py 98.80% <100.00%> (+4.96%) ⬆️

... and 1 file with indirect coverage changes

@TimKnight-Opencast TimKnight-Opencast marked this pull request as ready for review March 13, 2024 13:42
@TimKnight-Opencast TimKnight-Opencast marked this pull request as draft March 13, 2024 14:23
@TimKnight-Opencast TimKnight-Opencast marked this pull request as ready for review March 14, 2024 10:18
@amimas
Copy link
Collaborator

amimas commented Mar 17, 2024

Thanks for this PR @TimKnight-Opencast . Based on PR description and file changes, it seems branch protection processor is also included. Really appreciate the help converting both schedule processor and branch protection processor. But, it's a lot easier to review and safer if we keep the scope of changes to minimum. Could you please open a separate PR for branch protection processor related changes?

@amimas amimas changed the title feat: migrate schedules_processor to use python-gitlab refactor: migrate schedules_processor to use python-gitlab Mar 17, 2024
@TimKnight-Opencast
Copy link
Contributor Author

@amimas done I've cut a new branch for branch_processor changes.

Will raise a PR once the group_members refactor is in so can re-use the common python-gitlab code I'm adding in there

@TimKnight-Opencast
Copy link
Contributor Author

(same person, I just work for a consultancy at DWP, and due to stuff, needed to contribute from this account instead last week 😸 )

Copy link
Collaborator

@amimas amimas left a comment

Choose a reason for hiding this comment

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

It mostly looks good to me. Thanks for taking care of this. Only major issue I see is how the schedule is being created/updated.

gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/project/schedules_processor.py Outdated Show resolved Hide resolved
@TimKnight-Opencast
Copy link
Contributor Author

@amimas - reverted the test changes so it's just a subset using project_for_function (i.e. my new tests) and added that test for validating the logs when no changes to an existing schedule are made

@amimas
Copy link
Collaborator

amimas commented Apr 17, 2024

Thanks for all the work on this ❤️ . I've re-opened a conversation that I think is not resolved yet and it will likely introduce bug. Otherwise it looks good to merge.

@amimas amimas temporarily deployed to Integrate Pull Request April 19, 2024 02:24 — with GitHub Actions Inactive
Copy link
Collaborator

@amimas amimas 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. Thanks again for all the work.

@amimas amimas merged commit b8134e9 into gitlabform:main Apr 19, 2024
23 checks passed
@TimKnight-Opencast TimKnight-Opencast deleted the i-631 branch May 8, 2024 10:59
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.

Migrate schedules configuration feature to use python-gitlab library
3 participants