feat: do NOT skip presubmit optimizations for content hashing changes#4858
feat: do NOT skip presubmit optimizations for content hashing changes#4858jtmcdole merged 5 commits intoflutter:mainfrom
Conversation
|
test-app-dart is bad because the version of flutter updated I believe. see #4859 before I rebase this. |
| 'release-candidate-branch.version', | ||
| ); | ||
|
|
||
| static final _engineFilePaths = RegExp(r'^(DEPS|engine/.*|bin/internal/content_aware_hash\.(ps1|sh))$'); |
There was a problem hiding this comment.
This might be a bit overly paranoid, but if the name of the script is every changed, how will we remember to update it here?
There was a problem hiding this comment.
If the name of the script is ever changed:
- It will be a HUGE pain in the keister across multiple tools
- That PR would only land if someone was also changing the engine in that PR - for the reason I need this to land here (optimization would be triggered and pre submits would fail to run).
- If not # 2 (because DEPS or something else was changed) - the very next time someone changes the script it will fail for the same reasons as # 2 - so still safe.
I'm honestly not concerned with this.
jtmcdole
force-pushed
the
files_changed_op_incl_hash
branch
from
d9fe6e2 to
3bf3f1f
Compare
|
dangit. running dart analyze locally has no problem, its a breaking change somewhere else. |
…g changes this isn't an engine change, but we do need to build and test engines because the hash just isn't there. this is a rare condtion.
jtmcdole
force-pushed
the
files_changed_op_incl_hash
branch
from
3bf3f1f to
969f758
Compare
|
Yeah, so dart 3.9.4 is what's running in the workflow, while we have flutter stable at dart 3.9.2. I'll update the workflow to always use flutter setup. edit: and make sure the docker images that build use the right ones as well. |
| 'release-candidate-branch.version', | ||
| ); | ||
|
|
||
| static final _engineFilePaths = RegExp(r'^(DEPS|engine/.*|bin/internal/content_aware_hash\.(ps1|sh))$'); |
There was a problem hiding this comment.
Do we follow a common order of class members where fields are defined before methods?
There was a problem hiding this comment.
Not according to this code - the statics are here at the bottom.
|
Fixed some failing tests and pixel diffs. ACT should work locally if someone needs to use it (they should). I will investigate running nightly tests just to make sure down stream deps don't break us. |
3 participants
this isn't an engine change, but we do need to build and test engines because the hash just isn't there. this is a rare condition.