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

Refacto: move profiles rules filter to generic class and use it systematically in jobs #487

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

Guts
Copy link
Collaborator

@Guts Guts commented Apr 18, 2024

This PR is a following up of #481 and #486.

After some real-life testing of the profile deployment rules, it became clear that the initial implementation was clumsy.

By placing the filtering of profiles based on rules in the qprofiles-synchronizer job alone, we forget that other jobs based on the content of downloaded profiles also need to respect the rules: qplugins-synchronizer, etc.

This PR fixes this by moving the rules filtering logic
right into the generic job class and make sure that every job which relies on downloaded profiles use the same method to filter on rules.

@github-actions github-actions bot added the jobs Scenarios and jobs label Apr 18, 2024
@Guts Guts force-pushed the refacto/move-profiles-rules-filter-to-generic-class branch 3 times, most recently from d830b92 to a3dcb5f Compare April 19, 2024 14:42
@Guts Guts marked this pull request as ready for review April 19, 2024 19:33
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 70.19%. Comparing base (2a43596) to head (a0d5af7).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #487      +/-   ##
==========================================
- Coverage   70.64%   70.19%   -0.46%     
==========================================
  Files          48       48              
  Lines        3015     3013       -2     
  Branches      648      648              
==========================================
- Hits         2130     2115      -15     
- Misses        701      710       +9     
- Partials      184      188       +4     
Flag Coverage Δ
unittests 69.56% <63.15%> (-0.46%) ⬇️

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

Files Coverage Δ
...loyment_toolbelt/jobs/job_profiles_synchronizer.py 41.83% <66.66%> (-8.96%) ⬇️
qgis_deployment_toolbelt/jobs/job_splash_screen.py 67.85% <0.00%> (-10.72%) ⬇️
...ployment_toolbelt/jobs/job_plugins_synchronizer.py 73.01% <40.00%> (-4.41%) ⬇️
...deployment_toolbelt/jobs/job_plugins_downloader.py 50.38% <50.00%> (-2.30%) ⬇️
qgis_deployment_toolbelt/jobs/generic_job.py 89.15% <78.37%> (+6.39%) ⬆️

... and 3 files with indirect coverage changes

@Guts Guts self-assigned this Apr 19, 2024
@Guts Guts requested a review from jmkerloch April 19, 2024 20:16
@Guts Guts force-pushed the refacto/move-profiles-rules-filter-to-generic-class branch from 8a4dfb0 to a0d5af7 Compare April 22, 2024 20:01
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Guts Guts added the quality Tests, project resiliency, etc. label Apr 22, 2024
Copy link
Collaborator

@jmkerloch jmkerloch left a comment

Choose a reason for hiding this comment

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

LGTM !

@Guts Guts merged commit 1b76259 into main Apr 23, 2024
25 checks passed
@Guts Guts deleted the refacto/move-profiles-rules-filter-to-generic-class branch April 23, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jobs Scenarios and jobs quality Tests, project resiliency, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants