search

zulip / zulip-flutter

Upcoming Zulip mobile apps for Android and iOS, using Flutter
Apache License 2.0
202 stars 195 forks source link

check: Use --text when potentially diffing generated files #947

Closed PIG208 closed 2 months ago

PIG208 commented 2 months ago

The diffs generated from running tools/check build_runner were not detected. See comment: https://github.com/zulip/zulip-flutter/pull/904#discussion_r1762083210

We normally exclude diffs from generated files through our .gitattributes configurations. This happens to git --quiet -- [<path>...] too. -a/--text allows us to include those files regardless.

chrisbobbe commented 2 months ago

LGTM, marking for Greg's review.

gnprice commented 2 months ago

Aha, that diagnosis makes sense! Thanks @PIG208 for investigating and fixing this.

One thing that likely misled me, when I wrote the build_runner suite in the first place and was testing it locally, is that I have a .git/info/attributes which partly overrides the .gitattributes:

$ cat .git/info/attributes
*.g.dart diff
lib/host/*.g.dart -diff

In particular the diffs to our json_serializable-generated files under lib/api/ are fairly low-noise, and in review I like to see them to know what the deserialization code is actually doing.

So that would have caused the existing version of this code to work fine for me on those files, even while it didn't in CI or for other people locally.

The changes all look good; thanks for the nice commit message. Merging, with an added NFC commit on top.