Updated the analyze.dart deprecation check to only look for beta branch versions. #105290
Conversation
flutter-dashboard
bot
added
the
c: contributor-productivity
|
Adding @gspencergoog for Regex readability |
gspencergoog
left a comment
gspencergoog
left a comment
There was a problem hiding this comment.
In looking at this again, it looks like you don't need capture groups at all, much less named ones, for half of those regexps (except for the ones where you actually used namedGroup).
I don't recommend removing them though, because they serve as good built-in documentation. If speed/memory were a concern, I might say remove the groups, because they do have to be parsed out and kept, so there's a small cost, but in this case I think the documentation value is higher than the performance value.
christopherfujino
left a comment
christopherfujino
left a comment
There was a problem hiding this comment.
I'm still trying to understand this change, but from the PR description, I'm not sure this is the right change to make. Requesting changes for the moment while I do a more thorough review...
dev/bots/analyze.dart
Outdated
There was a problem hiding this comment.
beta release versions look like dev release versions, so I'm pretty sure we still need this logic, unless I'm misunderstanding the context for this change.
There was a problem hiding this comment.
I think the fix to make is to just s/dev/beta in the exception message
There was a problem hiding this comment.
Fair enough. I am not that familiar with this logic and build version numbers, so I am likely the one misunderstanding something 😄.
I was trying to land #105291 which has deprecations in it. I looked at the releases and the latest beta release is '3.1.0' which doesn't pass this check. When I asked about it on discord it was mentioned that perhaps we needed to update this check to support beta releases, so I thought that meant relaxing this check.
Any guidance here is appreciated. Thx.
There was a problem hiding this comment.
Ahh, oh yeah, so that was a one-time mistake that we named a beta release 3.1.0
There was a problem hiding this comment.
maybe we need to special case 3.1.0 in the test?
There was a problem hiding this comment.
Ok, I put back the previous logic (with some name cleanups) and special cased v3.1.0 so that it will pass. Please give this another look. Thx.
Also cleaned up a bunch of lint warnings for regexp raw strings.
…as it was mistakenly used as a beta version release.
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
…eta branch versions. (flutter/flutter#105290)
4 participants
The current analyzer bot script looks for dev versions in the deprecated warnings. As we no longer have dev releases this PR updates the check to only look for beta version numbers (i.e. vX.X.X).
This also cleans up some lint warnings for some of the regexp raw strings.
Pre-launch Checklist
///).