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: automatically remove prefix from scenario env vars #605

Merged
merged 8 commits into from
Jan 6, 2025

Conversation

Guts
Copy link
Collaborator

@Guts Guts commented Jan 6, 2025

This PR:

  • automatically remove QDT_ prefix from scenario settings name
  • log about it
  • global improvement of scenario reader regarding the dev guidelines
  • add missing recognized settings to JSON schemas
  • add a pattern to prevent end-user write a setting with the QDT_ prefix

@Guts Guts requested a review from jmkerloch January 6, 2025 11:38
@Guts Guts self-assigned this Jan 6, 2025
@Guts Guts marked this pull request as ready for review January 6, 2025 11:38
@github-actions github-actions bot added bug Something isn't working enhancement New feature or request labels Jan 6, 2025
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 46.42857% with 15 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
qgis_deployment_toolbelt/commands/deployment.py 33.33% 4 Missing and 4 partials ⚠️
...s_deployment_toolbelt/scenarios/scenario_reader.py 56.25% 4 Missing and 3 partials ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #605      +/-   ##
==========================================
- Coverage   70.26%   70.00%   -0.27%     
==========================================
  Files          47       47              
  Lines        3192     3204      +12     
  Branches      560      566       +6     
==========================================
  Hits         2243     2243              
- Misses        749      755       +6     
- Partials      200      206       +6     
Flag Coverage Δ
unittests 69.44% <46.42%> (-0.27%) ⬇️

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

Files with missing lines Coverage Δ
...s_deployment_toolbelt/scenarios/scenario_reader.py 54.54% <56.25%> (-5.46%) ⬇️
qgis_deployment_toolbelt/commands/deployment.py 64.86% <33.33%> (-5.73%) ⬇️

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jan 6, 2025
Copy link

sonarqubecloud bot commented Jan 6, 2025

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.

Good idea, this will avoid some confusion with environment variable.

@Guts
Copy link
Collaborator Author

Guts commented Jan 6, 2025

Pour tes étrennes @NROLLANDGrandlyon 😉

@Guts Guts merged commit 7ca5831 into main Jan 6, 2025
26 checks passed
@Guts Guts deleted the improve/scenario-prefixed-env-vars branch January 6, 2025 12:42
@NROLLANDGrandlyon
Copy link

Thanks guys !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants