Add pre-commit CI action #4982

Borda · 2021-09-28T23:03:28Z

this helps to maintain code formatting 🐰
moreover you may consider add this bot - https://github.com/marketplace/pre-commit-ci

🛠️ PR Summary

_{Made with ❤️ by Ultralytics Actions}

🌟 Summary

Introduce code formatting workflows and update minor comment syntax.

📊 Key Changes

Added a code formatting workflow using flake8 and pre-commit.
Minor white-space and code clean-up in various files for better syntax adherence.
Correction of typos and removal of trailing whitespace in documentation files.
Ensured consistency in Python scripts by using comprehensive list comprehensions and generators.

🎯 Purpose & Impact

🚀 Improves code readability and maintainability with automated styling checks.
🐞 Helps prevent syntax errors and facilitates future debugging.
💅 Promotes a consistent coding style throughout the project, improving collaboration.

Borda · 2021-09-29T15:43:06Z

@glenn-jocher mind have look? 🐰

glenn-jocher · 2021-09-30T06:03:21Z

@Borda thanks! Is there any way to move the new .yaml and .cfg files from the top level of the repo into a subdirectory?

Borda · 2021-09-30T07:06:47Z

@Borda thanks! Is there any way to move the new .yaml and .cfg files from the top level of the repo into a subdirectory?

I am not aware of it... do you have a reason not to have it in the root?

glenn-jocher · 2021-10-02T23:45:54Z

@Borda well that's a good question. I'm not a design or UX expert, but my main intention with the repo is to make everything as simple and easy as possible. I'd like to keep the root directory minimal to not scare away first time users (and hopefully to have each file in the root have an easily identifiable purpose at first glance). A minimal root with also increase the chances that the README banner will appear on page load without users having to scroll down too much, which may increase conversions (from viewers to >> cloners).

But I want to add linting at the same time! :)

In addition to these two PRs maybe we could set up another meeting so we could discuss the high level changes we should be pursuing, and then we can work out the details for these two PRs?

Borda · 2021-10-03T08:43:58Z

my main intention with the repo is to make everything as simple and easy as possible

well yes but if you want to have some settings in other folders instead of default root, you will add extra complexity instead and hope that this re-routing will work for everyone

keep the root directory minimal to not scare away first time users

Check for example sklearn - https://github.com/scikit-learn/scikit-learn how their root looks like
also do not see why it may scare anyone, I would be rather scared if there are too few things :D

A minimal root with also increase the chances that the README banner will appear on page load without users having to scroll down too much, which may increase conversions

I feel that is a kind of misunderstanding of the repo readme, if you want a landing page from readme GH can give it for you

In addition to these two PRs maybe we could set up another meeting so we could discuss the high level changes we should be pursuing, and then we can work out the details for these two PRs?

fine with me, also we can sen some "refactoring" GH project to fill tickets with expected work so also other contributors can help

Borda · 2021-10-03T18:44:01Z

@glenn-jocher Anyway, I would merge this one as entry gate if you agree...

Borda · 2021-10-13T13:20:12Z

@glenn-jocher can we merge this one?

Borda · 2021-10-21T22:45:49Z

@glenn-jocher not updating it anymore until you ask for it just before merging 🐰

glenn-jocher · 2021-10-22T18:19:33Z

@Borda hi there! Yes I'd like to get this merged. I see you've kept it up to date with master, thank you!!

Is there really no way to tidy up the root directory a bit? Can we shift any of the new files into .github or .github/workflows?

glenn-jocher · 2021-10-22T18:22:02Z

Oh BTW I implemented some updates to the rebase action (dependabot now updates actions, so dependabot updated to rebase.yml to v1.5, and I also replaced GITHUB_TOKEN with a new ACTIONS_TOKEN personal access token that includes workflows permissions), so it might be possible to rebase using the /rebase command now, but it still doesn't work for many PRs unfortunately.

EDIT: nope, seems like still failing with GitHub doesn't think that the PR is rebaseable! error
https://github.com/ultralytics/yolov5/runs/3979145773?check_suite_focus=true

Borda · 2021-10-22T18:40:56Z

Is there really no way to tidy up the root directory a bit? Can we shift any of the new files into .github or .github/workflows?

not really as pre-commit is an independent tool, not GH util
well, you can move the CONTRIBUTING.mb to .github

glenn-jocher · 2021-10-23T13:44:12Z

@Borda could you add a header to each of the 3 new files that explains what they do in a sentence or two? I think this would help everyone understand them better if users decide to open them and look around.

I see that pre-commit requires .pre-commit-config.yaml as they say in https://github.com/marketplace/pre-commit-ci, but they also say that nothing else is required beyond that. I'm guessing setup.cfg is required by Flake8 which is called by pre-commit?

Then the last file I'm a bit confused about, code-format.yml. Is this what actually runs pre-commit on PR commits?

BTW I think code-format.yml may have a problem, as it has branches: [main], but we are using master branch as the main branch, we haven't migrated over to the new naming.

Borda · 2021-10-26T16:09:30Z

I see that pre-commit requires .pre-commit-config.yaml as they say in https://github.com/marketplace/pre-commit-ci, but they also say that nothing else is required beyond that.

yes

I'm guessing setup.cfg is required by Flake8 which is called by pre-commit?

exactly

Then the last file I'm a bit confused about, code-format.yml. Is this what actually runs pre-commit on PR commits?

correct or you can activate this bot and then the new action is not needed, https://github.com/marketplace/pre-commit-ci

BTW I think code-format.yml may have a problem, as it has branches: [main], but we are using master branch as the main branch, we haven't migrated over to the new naming.

@glenn-jocher fixing it now

…-commit

glenn-jocher · 2021-10-28T16:35:45Z

@Borda PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐

Do you think we should add the bot at https://github.com/marketplace/pre-commit-ci, or should we wait a bit and see how this PR works?

Borda · 2021-10-28T16:58:28Z

Do you think we should add the bot at https://github.com/marketplace/pre-commit-ci, or should we wait a bit and see how this PR works?

I would rather use the bot as it pushes fixes to open PRs automatically even to forked PRs, compare to actions that works only with own PRs 🐰

* define pre-commit * add CI code * configure * apply pre-commit * fstring * apply MD * pre-commit * Update torch_utils.py * Update print strings * notes * Cleanup code-format.yml * Update setup.cfg * Update .pre-commit-config.yaml Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>

Borda mentioned this pull request Sep 28, 2021

use formatting Isort/YAPF/Black #4983

Closed

glenn-jocher assigned Borda Sep 29, 2021

Borda force-pushed the pre-commit branch 3 times, most recently from 15867b7 to 202f898 Compare September 29, 2021 15:41

Borda added 6 commits September 30, 2021 10:01

define pre-commit

1f38c56

add CI code

376d4d9

configure

a7b499f

apply pre-commit

6f4a88a

fstring

38d53c1

apply MD

cc51ff2

Borda force-pushed the pre-commit branch from 202f898 to cc51ff2 Compare September 30, 2021 08:03

Borda and others added 2 commits October 22, 2021 00:47

Merge branch 'master' into pre-commit

a42b9bf

pre-commit

2c82c2d

glenn-jocher added the TODO label Oct 23, 2021

glenn-jocher added 3 commits October 23, 2021 15:24

Update torch_utils.py

3928ad8

Update print strings

ab699c4

Merge master

c366075

Borda added 2 commits October 26, 2021 18:15

notes

acc5dfc

Merge branch 'pre-commit' of https://github.com/Borda/yolov5 into pre…

15baea1

…-commit

glenn-jocher changed the title ~~add pre-commit~~ Add pre-commit CI action Oct 28, 2021

glenn-jocher added 4 commits October 28, 2021 18:19

Cleanup code-format.yml

05ec5eb

Update setup.cfg

302f680

Update .pre-commit-config.yaml

aff631b

Merge upstream/master

1454ce1

glenn-jocher self-requested a review October 28, 2021 16:34

glenn-jocher approved these changes Oct 28, 2021

View reviewed changes

glenn-jocher merged commit ed887b5 into ultralytics:master Oct 28, 2021

glenn-jocher removed the TODO label Oct 28, 2021

Borda deleted the pre-commit branch October 28, 2021 16:57

glenn-jocher mentioned this pull request Feb 22, 2022

YOLOv5 v6.1 release #6739

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pre-commit CI action #4982

Add pre-commit CI action #4982

Borda commented Sep 28, 2021 •

edited by UltralyticsAssistant

Loading

Borda commented Sep 29, 2021

glenn-jocher commented Sep 30, 2021

Borda commented Sep 30, 2021

glenn-jocher commented Oct 2, 2021 •

edited

Loading

Borda commented Oct 3, 2021

Borda commented Oct 3, 2021 •

edited

Loading

Borda commented Oct 13, 2021

Borda commented Oct 21, 2021

glenn-jocher commented Oct 22, 2021

glenn-jocher commented Oct 22, 2021 •

edited

Loading

Borda commented Oct 22, 2021

glenn-jocher commented Oct 23, 2021 •

edited

Loading

Borda commented Oct 26, 2021 •

edited

Loading

glenn-jocher commented Oct 28, 2021

Borda commented Oct 28, 2021

Add pre-commit CI action #4982

Add pre-commit CI action #4982

Conversation

Borda commented Sep 28, 2021 • edited by UltralyticsAssistant Loading

🛠️ PR Summary

🌟 Summary

📊 Key Changes

🎯 Purpose & Impact

Borda commented Sep 29, 2021

glenn-jocher commented Sep 30, 2021

Borda commented Sep 30, 2021

glenn-jocher commented Oct 2, 2021 • edited Loading

Borda commented Oct 3, 2021

Borda commented Oct 3, 2021 • edited Loading

Borda commented Oct 13, 2021

Borda commented Oct 21, 2021

glenn-jocher commented Oct 22, 2021

glenn-jocher commented Oct 22, 2021 • edited Loading

Borda commented Oct 22, 2021

glenn-jocher commented Oct 23, 2021 • edited Loading

Borda commented Oct 26, 2021 • edited Loading

glenn-jocher commented Oct 28, 2021

Borda commented Oct 28, 2021

Borda commented Sep 28, 2021 •

edited by UltralyticsAssistant

Loading

glenn-jocher commented Oct 2, 2021 •

edited

Loading

Borda commented Oct 3, 2021 •

edited

Loading

glenn-jocher commented Oct 22, 2021 •

edited

Loading

glenn-jocher commented Oct 23, 2021 •

edited

Loading

Borda commented Oct 26, 2021 •

edited

Loading