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

MAINT: Ignore black format changes #713

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

jarrodmillman
Copy link
Collaborator

@jarrodmillman jarrodmillman commented Jun 11, 2022

See the comment below for an explanation of this: #713 (comment)

@jarrodmillman jarrodmillman added this to the 0.9.1 milestone Jun 11, 2022
@choldgraf
Copy link
Collaborator

choldgraf commented Jun 11, 2022

Can you please provide a little bit more context of the "what and the why"? I don't understand what this diff means, or why it is important to "ignore black format changes". It seems reasonable to me but I don't really know why I think that haha, I think it'd be helpful for others to understand and learn if we had a bit more info.

Is the goal here to prevent github from having noise about changes that were related to a bot? Is there some way we can automate this process? Or do we have to manually update it each time?

@jarrodmillman
Copy link
Collaborator Author

jarrodmillman commented Jun 11, 2022

To see how this works, consider this commit

That commit touched several files, but mostly applied autoformatting changes. When you use git blame at the commandline or if you use the blame view in the GitHub interface you get
screenshot

See
https://github.com/pydata/pydata-sphinx-theme/blame/main/docs/conf.py

After merging this PR you would get
screenshot

See
https://github.com/jarrodmillman/pydata-sphinx-theme/blame/git-blame-ignore-revs/docs/conf.py

In particular, you can see the difference by comparing lines 7--9.

I am not aware of any automated tool to do this, but it isn't a problem and the commits can be added to the file at any time.

I don't think this is super important. Personally, I don't have any trouble ignoring the handful of formatting commits when using git blame. But it is mildly helpful and may reduce concerns about using autoformatters like black, pyupgrade, autopep8, etc.

I would be happy if this was merged, but would also be happy if it wasn't.

@choldgraf
Copy link
Collaborator

I'm +1 on merging this, as long as we either:

  • Can put a comment in the .git-blame-ignore-revs file that explains what it is for, maybe even just with a link to this PR.
  • Put a little section in our contributing docs to document this.

I just don't want to add a dangling file that many people might not be familiar with without also adding a little bit of documentation :-)

Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for adding these extra explanations and links :-)

@choldgraf choldgraf changed the title Ignore black format changes MAINT: Ignore black format changes Jun 16, 2022
@choldgraf choldgraf merged commit 13dc608 into pydata:main Jun 16, 2022
@jarrodmillman jarrodmillman modified the milestones: 0.9.1, 0.10 Jul 1, 2022
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.

3 participants