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

Add flake8 bugbear #22

Closed
wants to merge 1 commit into from
Closed

Add flake8 bugbear #22

wants to merge 1 commit into from

Conversation

Mews
Copy link
Contributor

@Mews Mews commented Jun 10, 2024

Created this pull request in regards to issue #9

Changes

  • Added flake8-bugbear to requirements-dev.txt so that it is installed when linting
  • Added command in the linting.yml workflow to verify flake8 is picking up flake8-bugbear

Summary by CodeRabbit

  • New Features

    • Introduced a GitHub Actions workflow for code coverage reporting.
    • Added a Codecov badge to the README for visibility of coverage status.
  • Chores

    • Updated development dependencies to include coverage, flake8-bugbear, and pytest-cov.
    • Enhanced linting configuration to ensure flake8-bugbear is properly utilized.

Copy link
Contributor

coderabbitai bot commented Jun 10, 2024

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Files selected but had no reviewable changes (1)
  • tox.ini

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This update enhances the development and continuous integration process for the Seluj78/flask-utils repository. Key changes include the addition of flake8-bugbear verification in linting, new dependencies in requirements-dev.txt, a new GitHub Actions workflow for coverage using Codecov, and a Codecov badge in the README. These improvements aim to ensure code quality and provide better insights into test coverage.

Changes

File Change Summary
.github/workflows/linting.yml Added a verification step for flake8 to ensure it picks up flake8-bugbear by running flake8 --version.
requirements-dev.txt Added new packages: coverage, flake8-bugbear, and pytest-cov. Maintained existing packages.
.github/workflows/coverage.yml Introduced a new workflow named "Coverage" that runs on PRs and pushes to the main branch, using tox and uploading reports to Codecov.
README.md Added Codecov badge to display test coverage status.

Poem

In code we trust, with linting tight,
Flake8-bugbear joins the fight.
Coverage reports, now we see,
With badges shining, proud and free.
Dependencies strong, our tests will run,
In Python's world, our work is done.
🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Seluj78 Seluj78 linked an issue Jun 10, 2024 that may be closed by this pull request
@Seluj78 Seluj78 added this to the v1 milestone Jun 10, 2024
@Seluj78 Seluj78 added the enhancement New feature or request label Jun 10, 2024
@Seluj78 Seluj78 self-requested a review June 10, 2024 14:14
@Seluj78
Copy link
Owner

Seluj78 commented Jun 10, 2024

Hi there ! Thank you for the PR @Mews !

On the code side, it looks great to me ! 🙌

As for the commits, I would like you to adhere to the guideline (you can use scripts/lint-commits.sh to check if they match).

Also I prefer to use rebase instead of merge to update branches. Could you do this and then we'll be able to merge your PR 🔥

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cf29e8d and 46be363.

Files selected for processing (2)
  • .github/workflows/linting.yml (1 hunks)
  • requirements-dev.txt (1 hunks)
Files skipped from review due to trivial changes (2)
  • .github/workflows/linting.yml
  • requirements-dev.txt

coderabbitai[bot]
coderabbitai bot previously approved these changes Jun 10, 2024
@Mews
Copy link
Contributor Author

Mews commented Jun 10, 2024

Hi, sorry but this is my first time opening a pr and I'm still learning this git stuff so I'm not entirely sure how to fix those.
Did I understand correctly that the commit messages should follow some sort of format? If so, is there some way to edit the commit messages or how do I proceed?
Also I don't know what you mean by using rebase instead of merge.
Sorry for the trouble

@Seluj78
Copy link
Owner

Seluj78 commented Jun 10, 2024

Hi, sorry but this is my first time opening a pr and I'm still learning this git stuff so I'm not entirely sure how to fix those. Did I understand correctly that the commit messages should follow some sort of format? If so, is there some way to edit the commit messages or how do I proceed? Also I don't know what you mean by using rebase instead of merge. Sorry for the trouble

Hey no worries, I'm happy to help ! We all had to learn at some point !

So yes that is correct. I personally like my commits to follow a format, which you can find described here. You can also find examples of commit messages following the rules here

As for editing a commit, I recommend you search online either how to do an interactive rebase (git rebase -i) or use a UI editor which would allow you do to this easily.

Same thing for merge vs rebase, I advise you search online for what the difference are and how you can rebase your branch onto main :)

I am not trying to be rude/lazy by not responding to your questions directly :) The biggest thing you can learn to be a good software engineer is searching yourself ;) Do let me know if you get stuck

@Mews
Copy link
Contributor Author

Mews commented Jun 10, 2024

@Seluj78
Maybe I'm wrong but it looks like I can only change the messages on commits that haven't been pushed?
I tried using git commit --amend -m "the message..." and then git rebase --continue but it didn't change any message, and then I tried using github desktop but it looks like there's no option for that either

Also I'm a bit confused about what you said about preferring rebase instead of merge to update branches.
I probably understood it wrong, but isn't that something that you would have to do on your side?

Also, I was gonna change the commit messages to the following
CI/CD: Verify flake8 is picking up flake8-bugbear
and
Chore: Add flake8-bugbear to dev requirements
I couldn't really figure out how to run the lint-commits.sh script so I'd just like to confirm that these messages are okay

@Mews
Copy link
Contributor Author

Mews commented Jun 10, 2024

Update: I think I figured out how to do it from this article https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/changing-a-commit-message#commit-has-not-been-pushed-online

However, doing git rebase -i HEAD~3 opens some sort of text editor. I am able to replace pick with reword as per the article's instructions, but then I dont know how to save the file

@Mews
Copy link
Contributor Author

Mews commented Jun 10, 2024

@Seluj78
Okay I managed to change the commit messages!
Now I just need help with that rebase vs merge thing

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range and nitpick comments (1)
README.md (1)

Line range hint 69-69: Please add blank lines around the fenced code blocks to improve readability and adhere to Markdown best practices.

Also applies to: 74-74, 79-79, 86-86

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 46be363 and d9ae02a.

Files selected for processing (6)
  • .github/workflows/coverage.yml (1 hunks)
  • .github/workflows/linting.yml (1 hunks)
  • README.md (1 hunks)
  • codecov.yml (1 hunks)
  • requirements-dev.txt (1 hunks)
  • tox.ini (1 hunks)
Files skipped from review due to trivial changes (3)
  • .github/workflows/coverage.yml
  • .github/workflows/linting.yml
  • codecov.yml
Additional context used
LanguageTool
README.md

[style] ~1-~1: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 1067 characters long) (EN_EXCESSIVE_EXCLAMATION)
Context: Documentation Status GitHub commit activity GitHub commits since latest release GitHub Actions Workflow Status GitHub Actions Workflow Status ![GitHub License](https://img.shields.io/...


[misspelling] ~85-~85: This word is normally spelled as one. (EN_COMPOUNDS_MULTI_THREADED)
Context: ...ests bash tox OR Run the tests multi-threaded bash tox -p

Markdownlint
README.md

69-69: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines


74-74: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines


79-79: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines


86-86: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines

Additional comments not posted (6)
requirements-dev.txt (3)

3-3: The addition of coverage is appropriate for enhancing code coverage metrics.


5-5: Including flake8-bugbear is a good choice for catching subtle bugs and bad code practices.


10-10: Adding pytest-cov is beneficial for generating detailed test coverage reports.

tox.ini (2)

15-18: The configuration of the clean environment is correctly set up to erase previous coverage data, ensuring accurate coverage reports.


20-27: The setup of the coverage environment for running coverage analysis with pytest is correctly configured.

README.md (1)

7-7: The addition of the Codecov badge is a good practice, enhancing visibility into the project's code coverage metrics.

@Seluj78
Copy link
Owner

Seluj78 commented Jun 10, 2024

@Seluj78 Okay I managed to change the commit messages! Now I just need help with that rebase vs merge thing

That's great !

Indeed, you can do an interactive rebase to reword the commits like you did, which is perfect ! And yes, renaming your latest commit is easier since you can simply do git commit --amend :)

Now as for rebase vs merge, if you look at https://github.com/Seluj78/flask-utils/pull/22/commits and https://github.com/Seluj78/flask-utils/pull/22/files you can see that you have merged into this branch extra stuff that is not needed. A merge will add the changes on top of what you've made, whereas a rebase will place your new changes on top, which is what we want to have a nice history.

What you need to do it checkout your main branch, update it from my remote (search on google how to update forked remote branch), then checkout your branch against, and do a git rebase main and it will do it magically. Then all you need to do is git push --force. More info can be found here: https://git-scm.com/book/en/v2/Git-Branching-Rebasing

@Mews
Copy link
Contributor Author

Mews commented Jun 10, 2024

I don't think it did what it was supposed to ngl
I did git checkout main git fetch upstream git rebase upstream/main git checkout add-flake8-bugbear git rebase main and git push --force

@Seluj78
Copy link
Owner

Seluj78 commented Jun 10, 2024

Hmm indeed it does seem to have removed your changes. Do you remember what you did to do it again ? What I would advise you to do is:

  • Close this PR
  • Delete the branch
  • Checkout main
  • run git log and compare it to https://github.com/Seluj78/flask-utils/commits/main/ to make sure you have the same thing (same commits)
  • If that's the case, then you can create the branch again and do your changes, commit again and reopen the PR !

@Mews
Copy link
Contributor Author

Mews commented Jun 10, 2024

Yeah I'll just make the branch again :P

@Mews Mews closed this Jun 10, 2024
@Mews Mews deleted the add-flake8-bugbear branch June 10, 2024 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use flake8-bugbear
2 participants