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

Added bump version and pre-commit #107

Merged
merged 2 commits into from
Feb 14, 2022

Conversation

marksweb
Copy link
Sponsor Member

@marksweb marksweb commented Jan 23, 2022

Description

Added bump version and pre-commit to help with code quality and releasing. This forms part of changes to improve the developer experience, to make everything easier.

Bump version will automatically bump the package version and update the change log for the release from the unreleased changes.

This explains bump version fairly well if you're unfamiliar

https://williamhayes.medium.com/versioning-using-bumpversion-4d13c914e9b8

Checklist

  • I have opened this pull request against master
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined #workgroup-pr-review on
    Slack to find a “pr review buddy” who is going to review my pull request.

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2022

Codecov Report

Merging #107 (d3bc204) into master (883f9f8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #107   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          166       166           
  Branches         7        23   +16     
=========================================
  Hits           166       166           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 883f9f8...d3bc204. Read the comment docs.

setup.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Aiky30 Aiky30 left a comment

Choose a reason for hiding this comment

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

Not sure I like the requirements.txt, prefer to manually add them but interested and open to see different techniques and how they work in the longer term. If we don't move on we would still be patching JS for IE5 & 6, they were horrible times.

@marksweb
Copy link
Sponsor Member Author

Not sure I like the requirements.txt, prefer to manually add them but interested and open to see different techniques and how they work in the longer term. If we don't move on we would still be patching JS for IE5 & 6, they were horrible times.

Yeah I get that, but it should be a familiar sight that people know what to do with. If you're using pycharm it'll even offer you the venv install dialog when you first open the projects with the requirements file installed by default so it makes for a smoother workflow.

We could list the packages required to make changes in the readme/docs but they might not be read and one of the key parts of this new pattern is pre-commit which needs to be installed and the hooks need adding to the repo otherwise those checks only get caught at the PR.

So I think it's a clear way of laying things out, and for anybody that knows the packages in there, like pre-commit, they'll immediately know what they're doing (hopefully).

@marksweb marksweb merged commit a56091c into django-cms:master Feb 14, 2022
@marksweb marksweb deleted the feature/bump-version branch February 14, 2022 20:10
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.

None yet

4 participants