-
Notifications
You must be signed in to change notification settings - Fork 19
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
Updating met repo with all changes on epic engage #2348
Conversation
@VineetBala-AOT Thanks for submitting this with the great summary. I've moved it to a draft PR just to prevent it from being merged accidentally. FYI this PR has been submitted for review only at the moment. Vineet has said this includes general improvements brought over from the Epic project. Could members of the DES team please not approve/merge this PR until Steve and I have had a chance to review it. |
@@ -36,7 +37,7 @@ | |||
os.environ = {k: v for k, v in os.environ.items() if v} | |||
|
|||
|
|||
def get_named_config(environment: str | None) -> 'Config': | |||
def get_named_config(environment: Union[str, None]) -> 'Config': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of the team members have had issues with this. Do you know what the underlying problem is? Is the Python version not recognizing the union type hint syntax?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I believe its because of the python version.
""" | ||
Initialize the object. | ||
|
||
This method is called when an object is created. It sets up the initial |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments are always appreciated! Especially to add more context to an existing file
|
||
# TODO: Populate or remove this method dependent on changes resulting from adding the new Engagement metadata | ||
return query | ||
# TODO: Populate or remove this method dependent on changes resulting from adding the new Engagement metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Nice of you to fix little issues like this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for merging this in, @VineetBala-AOT. It's especially nice that you corrected preexisting linting errors and did some small code clean ups when you didn't have to. Appreciated!
Could you please just update the CHANGELOG? Please follow the current format of the log, with the date as header, then a brief description of the work and a link to the ticket. I've created a ticket to capture this work in Jira: https://apps.itsm.gov.bc.ca/jira/browse/DESENG-468
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2348 +/- ##
==========================================
+ Coverage 68.56% 68.90% +0.33%
==========================================
Files 472 510 +38
Lines 15593 15968 +375
Branches 1193 1165 -28
==========================================
+ Hits 10691 11002 +311
- Misses 4693 4757 +64
Partials 209 209
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
* Changes to show all survey results to superusers * removing hard coded values * fixing linting * splitting to seperate end points * Handle no data error for graphs * adding new nodata component * adding new email for submission response * fixing linting and testing * Upgrades to Issue Tracking Table * removing try catch * removing changes for email verification * updating * Merging bug fixes from epic engage * Updating comment service to remove metadata * fixing met-api linting * Updating change log * Updating comment * fixing linting for analytics
Issue #: https://github.com/bcgov/met-public/issues/
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).