-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
EWS should not be triggered for multi-year old patches #2807
EWS should not be triggered for multi-year old patches #2807
Conversation
bbb9898
to
7cb45a1
Compare
change_id = int(change_id) | ||
except: | ||
return HttpResponse('Invalid patch id provided, should be an integer. git hashes are not supported.') | ||
|
||
_log.info('SubmitToEWS::change: {}'.format(change_id)) | ||
if change_id < 430000: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
430000 is around May-2021. We can adjust it if anyone feel it's not very appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's possible to fetch the latest patch date, that would be more ideal since this blocks old bugs from being picked up again and tested within Bugzilla. (but since we want to deprecate this anyways, I am not sure)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and by picked up I mean re-submitted as a patch to the same Bugzilla.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible but Alexey had another good point about ChangeLog. Went with that.
I'm not sure if I agree with the premise - it seems legitimate to check if old patches still work, and they sometimes may. However, we are in an exceptional situation now, where none of old patches apply because of ChangeLogs. So if we do this, the cut-off date should be when we dropped ChangeLogs. |
7cb45a1
to
75c62bf
Compare
Agree. Updated to change the cut-off near to the date when we dropped ChangeLogs. |
if change_id < 459500: | ||
return HttpResponse('Patch is too old. Skipping.') | ||
|
||
if change_id > 600000: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting a time limit on deprecating patch based workflows, eh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah 😆
Did very rough calculations. It's 10+ year in future based on current patch rate.
Just wanted to filter any garbage input while I was at it.
https://bugs.webkit.org/show_bug.cgi?id=243271 Reviewed by Ryan Haddad. * Tools/CISupport/ews-app/ews/views/submittoews.py: (SubmitToEWS.post): Canonical link: https://commits.webkit.org/252916@main
75c62bf
to
6e27809
Compare
Committed 252916@main (6e27809): https://commits.webkit.org/252916@main Reviewed commits have been landed. Closing PR #2807 and removing active labels. |
6e27809