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 header workflow integration 2 #5109
Add header workflow integration 2 #5109
Conversation
eb7d6e2
to
122cef8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the comments I have already left, I noticed that some of the emails in the add-header.py
:
Line 52 in d59c237
("Alessandro Di Girolamo", "<digirola@lxplus030.cern.ch>"): ("Ale Di Girolamo", "<alessandro.di.girolamo@cern.ch>"), |
Line 74 in d59c237
("rcarpa", "<2905943+rcarpa@users.noreply.github.com>"): ("Radu Carpa", "<radu.carpa@cern.ch>"), |
look weird/temporary. I guess not all commits by these authors come from the same email address. Are we sure that we should be using these email addresses? WDYT @rcarpa @bari12
9e048ba
to
c4b995c
Compare
Is it strictly needed to re-touch every file (422 changes) or can't we just get this up to date over time? |
@maany and I discussed that, and the problem is that you have to get the diff between the PR and the master branch. I tried that first, but the commit SHA provided by GitHub Actions is not working properly when a commit is overwritten via a I also don't know anyone who has experience with that particular issue... |
8e106ef
to
5ae6a50
Compare
f490888
to
dde77e1
Compare
Okay, after @bari12 gave some hints on how to get the changed files in GItHub Actions the implementation now only checks the headers of the changed python files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @joeldierkes ! Awesome to see we don't need to parse all the files on every pull request. I've left some minor comments. ;)
I really like the new version. Nice job! |
8d3e6a0
to
9c163a2
Compare
The `add_header` script needs to be run after an edit to determine the status of the header and, if necessary, change the header. This commit adds the check for the header to the CI/CD. This enforces the usage of the `add_header` script and ensures a healthy code base.
The `add_header` sript prints the correct header for a file if the `--in-place` argument is not provided. This is a good way to see changes, however due to this the output of the CI/CD is too verbose. This commit disables the output of the output of the correct header, if the `--check` option is provided. It also displays the number of files checked.
9c163a2
to
c305e83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Add a header check to the CI Fix #4980 The `add_header` script needs to be run after an edit to determine the status of the header and, if necessary, change the header. This commit adds the check for the header to the CI/CD. This enforces the usage of the `add_header` script and ensures a healthy code base. * Change the output of the `add_header` script Fix #4980 The `add_header` sript prints the correct header for a file if the `--in-place` argument is not provided. This is a good way to see changes, however due to this the output of the CI/CD is too verbose. This commit disables the output of the output of the correct header, if the `--check` option is provided. It also displays the number of files checked.
* Add a header check to the CI Fix rucio#4980 The `add_header` script needs to be run after an edit to determine the status of the header and, if necessary, change the header. This commit adds the check for the header to the CI/CD. This enforces the usage of the `add_header` script and ensures a healthy code base. * Change the output of the `add_header` script Fix rucio#4980 The `add_header` sript prints the correct header for a file if the `--in-place` argument is not provided. This is a good way to see changes, however due to this the output of the CI/CD is too verbose. This commit disables the output of the output of the correct header, if the `--check` option is provided. It also displays the number of files checked.
This commit adds a header check to the github actions.
As of right now, the headers get checked on every push. We could change that to every pull-request if required.
See individual commits for more information.