Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add linguist-generated to gitattributes #13628

Merged
merged 4 commits into from
Mar 18, 2023

Conversation

pgherveou
Copy link
Contributor

This will keep generated weights.rs from displaying in diffs by default
see https://docs.github.com/en/repositories/working-with-files/managing-files/customizing-how-changed-files-appear-on-github
for more details

@pgherveou pgherveou added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 17, 2023
@ggwpez
Copy link
Member

ggwpez commented Mar 17, 2023

Then how do we review them?

@pgherveou
Copy link
Contributor Author

Then how do we review them?

They still show up in the file list, but are by default collapsed, and they don't affect the PR diff numbers, (so people are not scared by a big number when they review your PR)

That's the theory, can try to test this out with this PR to make sure that works as intended

@ggwpez
Copy link
Member

ggwpez commented Mar 17, 2023

That's the theory, can try to test this out with this PR to make sure that works as intended

Less trust, more truth. Let's go!

@ggwpez
Copy link
Member

ggwpez commented Mar 17, 2023

bot bench $ pallet dev pallet_balances
(Bot is probably a bit slow since it i used in other MRs)

@command-bot
Copy link

command-bot bot commented Mar 17, 2023

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2542934 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_balances. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 79-fd955845-a4e5-477a-9a8d-07cd5e60aa91 to cancel this command or bot cancel to cancel all commands in this pull request.

@pgherveou
Copy link
Contributor Author

pgherveou commented Mar 17, 2023

noobs question, would that bot call actually create a diff, since this PR has no changes?
I can also just touch a file by hand

@pgherveou pgherveou removed B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 17, 2023
@pgherveou pgherveou added B0-silent Changes should not be mentioned in any release notes D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Mar 17, 2023
@ggwpez
Copy link
Member

ggwpez commented Mar 17, 2023

noobs question, would that bot call actually create a diff, since this PR has no changes?

Yes it should. And yes; modifying by hand is also possible, but I am lazy and then you would have to revert it 😛
This way we can see that it works end-to-end.

@command-bot
Copy link

command-bot bot commented Mar 17, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_balances has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2542934 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2542934/artifacts/download.

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

This is how it looks now, seems fine 👍
image

.gitattributes Outdated Show resolved Hide resolved
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@pgherveou pgherveou merged commit 29466db into master Mar 18, 2023
@pgherveou pgherveou deleted the pg/add-linguist-generated-to-gitattributes branch March 18, 2023 07:44
@athei
Copy link
Member

athei commented Mar 19, 2023

and they don't affect the PR diff numbers, (so people are not scared by a big number when they review your PR)

This part seems to be wrong :( Look at the diff stats of this PR. It includes the generated file. Other PRs which include this change also have scary diff stats: #13312

The description of the feature also never mentions that it will be ignored in PR diff stats. Only in the PRs repository's language statistics. So it does seem to be intentional. I wish there were a way to hide it from PR diff stats, though.

@pgherveou
Copy link
Contributor Author

and they don't affect the PR diff numbers, (so people are not scared by a big number when they review your PR)

This part seems to be wrong :( Look at the diff stats of this PR. It includes the generated file. Other PRs which include this change also have scary diff stats: #13312

The description of the feature also never mentions that it will be ignored in PR diff stats. Only in the PRs repository's language statistics. So it does seem to be intentional. I wish there were a way to hide it from PR diff stats, though.

yep saw that as well, this is not that useful after all...

breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants