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

Update workflow and deps #111

Merged
merged 15 commits into from
May 16, 2023
Merged

Update workflow and deps #111

merged 15 commits into from
May 16, 2023

Conversation

JamesDearlove
Copy link
Member

@JamesDearlove JamesDearlove commented May 5, 2023

Updating workflow dependencies, and also package dependencies as well.

While I was here, I removed the channel cog and dealt with some quick deprecation warnings (ie SQLAlchemy 2)

Closes #105

Update 10th May I've also now gone and:

  • Added a Pytest workflow to run Pytest on PRs
  • Removed Docker build workflow from PRs
  • Rewritten Dockerfile to be multi-stage, which should speed up deployment
  • Fixed @andrewj-brown's merge issue

Copy link
Member

@andrewj-brown andrewj-brown left a comment

Choose a reason for hiding this comment

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

LGTM conditional on the #94 changes being rebased in.

I assume this'll fix all our deprecation warnings? I can't see anything that touches save-state or set-output, unless they're pulled in via the deps you've updated.

@JamesDearlove
Copy link
Member Author

I assume this'll fix all our deprecation warnings? I can't see anything that touches save-state or set-output, unless they're pulled in via the deps you've updated.

Yeah all those seemingly were part of the outdated deps, which is very convenient

Copy link
Member

@andrewj-brown andrewj-brown left a comment

Choose a reason for hiding this comment

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

lgtm still. you've reminded me to do a wider documentation review at some point, cause rn it's all pretty shit

We use an automated code formatter called [Black](https://black.readthedocs.io/), currently this needs to be run manually to pass the format CI check. To run Black, run from the root of the repo:

```bash
poetry run black uqcsbot
Copy link
Member

Choose a reason for hiding this comment

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

does that work? poetry just inherently knows which files to do? massive

Copy link
Member Author

Choose a reason for hiding this comment

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

Well in this case, its running it on the folder uqcsbot so it's smart enough to know what files to run it on otherwise

@JamesDearlove
Copy link
Member Author

I've also now gone and:

  • Added a Pytest workflow to run Pytest on PRs
  • Removed Docker build workflow from PRs
  • Rewritten Dockerfile to be multi-stage, which should speed up deployment
  • Fixed @andrewj-brown's merge issue

@JamesDearlove JamesDearlove merged commit 042c7bb into main May 16, 2023
@andrewj-brown andrewj-brown deleted the jimmy-updates branch July 1, 2023 00:55
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.

kill joinchannel cog
2 participants