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

Maintenance week updates #206

Merged
merged 5 commits into from
Dec 7, 2023
Merged

Maintenance week updates #206

merged 5 commits into from
Dec 7, 2023

Conversation

ehanson8
Copy link
Contributor

@ehanson8 ehanson8 commented Dec 6, 2023

What does this PR do?

Updates app according to our maintenance week documentation

How can a reviewer manually see the effects of these changes?

Run make run-dev to run the app against our Alma sandbox API. This doesn't have any data to generate credit card slips but you can see that app executes properly and sends a notification email to our dev1 address

I also verified the changes by running manually against Alma prod for 2023-12-04 and observed that the email attachment matched today's email attachment for the same date.

Includes new or updated dependencies?

YES

Developer

  • All new ENV is documented in README (or there is none)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

* Replace setup.cfg with pyproject.toml
* Uninstall legacy linters and install current linters
* Add new linting commands to Makefile
* Add help command to Makefile
* Install pre-commit and add config yaml
* Updates to type hinting, code structure, datetime objects, unit tests, fixtures, and docstrings as requested by linters
Copy link

@ghukill ghukill 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 for this work.

tests/test_cli.py Outdated Show resolved Hide resolved
tests/test_email.py Show resolved Hide resolved
Makefile Show resolved Hide resolved
Copy link
Contributor

@jonavellecuerdo jonavellecuerdo left a comment

Choose a reason for hiding this comment

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

1 small request for change! :) Otherwise, everything looks good!

Pipfile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
* Remove lambda-related Makefile commands
* Replace caplog.at_level with caplog.set_level in CLI test
* Remove coverage from Pipfile
Copy link

github-actions bot commented Dec 6, 2023

Pull Request Test Coverage Report for Build 7119503500

  • 23 of 23 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 6566213700: 0.0%
Covered Lines: 192
Relevant Lines: 192

💛 - Coveralls

@ehanson8 ehanson8 merged commit 603d196 into main Dec 7, 2023
3 checks passed
@ehanson8 ehanson8 deleted the maintenance-week-updates branch December 7, 2023 15:56
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.

3 participants