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

Using f-strings and log on import #28

Merged
merged 12 commits into from Oct 13, 2022
Merged

Conversation

ehsteve
Copy link
Member

@ehsteve ehsteve commented Sep 30, 2022

This cleans up the code to use fstrings instead of the format syntax. fstrings make the code much cleaner. Also updated the code standards to say to use fstrings.

This also adds a log message on import to show the version number of the package on import. Useful for debugging purposes.

Addresses issues #27, #23.

@ehsteve ehsteve requested a review from dbarrous October 6, 2022 13:18
Copy link
Member

@dbarrous dbarrous left a comment

Choose a reason for hiding this comment

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

Overall looks good to me! Definitely prefer this formatting over the .format(), way easier to read and make sense of. Did notice two little nitpicks but other than that ✅

hermes_core/__init__.py Outdated Show resolved Hide resolved
hermes_core/util/util.py Outdated Show resolved Hide resolved
@dbarrous
Copy link
Member

Also just noticed there a few merge conflicts, you might want to re-sync your branch with main or if you give me the go ahead I can resolve them for you ^

@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #28 (fc61996) into main (4e4a029) will increase coverage by 0.11%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
+ Coverage   75.96%   76.07%   +0.11%     
==========================================
  Files           5        5              
  Lines         208      209       +1     
==========================================
+ Hits          158      159       +1     
  Misses         50       50              
Impacted Files Coverage Δ
hermes_core/util/config.py 55.81% <0.00%> (ø)
hermes_core/util/util.py 90.90% <83.33%> (+0.13%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

This was linked to issues Oct 13, 2022
@ehsteve ehsteve merged commit ed6eb01 into HERMES-SOC:main Oct 13, 2022
@ehsteve ehsteve deleted the format_remove branch October 13, 2022 17:13
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.

Update to use f-strings Add log output on import
2 participants