Conditionally disable the patch validation step
Created attachment 412908 [details] Patch
Comment on attachment 412908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412908&action=review > Tools/ChangeLog:11 > + Note that the skip_validation parameter cannot be supplied remotely, so This build property can be supplied while using 'buildbot try' command (which ews-app uses). So, this note doesn't seems accurate. > Tools/CISupport/ews-build/steps.py:661 > + return int(self.getProperty('skip_validation', 0)) == 0 should we make this property boolean instead of 0/1? Something like: return not self.getProperty('skip_validation', False)
(In reply to Aakash Jain from comment #2) > Comment on attachment 412908 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=412908&action=review > > > Tools/ChangeLog:11 > > + Note that the skip_validation parameter cannot be supplied remotely, so > > This build property can be supplied while using 'buildbot try' command > (which ews-app uses). So, this note doesn't seems accurate. My bad. 'buildbot try' can of course connect to a remote server instance. What I meant to say is that, AFAIU, Try_Userpass uses a separate set of auth credentials, so that one can only set skip_validation by going through the ews-app. If that's not the case, do you have a suggestion for good way to skip patch validation for testing setups? > > Tools/CISupport/ews-build/steps.py:661 > > + return int(self.getProperty('skip_validation', 0)) == 0 > > should we make this property boolean instead of 0/1? Something like: > > return not self.getProperty('skip_validation', False) I actually selected this because it's probably easier for the user to figure out how to specify a boolean property to the buildbot cli client but, as it's unlikely this would be used directly by someone, using a bool seems like an improvement. I'll submit an updated patch. Thanks!
(In reply to Angelos Oikonomopoulos from comment #3) > (In reply to Aakash Jain from comment #2) > > Comment on attachment 412908 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=412908&action=review > > > > > Tools/ChangeLog:11 > > > + Note that the skip_validation parameter cannot be supplied remotely, so > > > > This build property can be supplied while using 'buildbot try' command > > (which ews-app uses). So, this note doesn't seems accurate. > > My bad. 'buildbot try' can of course connect to a remote server instance. > What I meant to say is that, AFAIU, Try_Userpass uses a separate set of auth > credentials, so that one can only set skip_validation by going through the > ews-app. > > If that's not the case, do you have a suggestion for good way to skip patch validation for testing setups? My comment was only about the Note in ChangeLog, the logic looks fine.
Created attachment 413167 [details] Patch
> > My bad. 'buildbot try' can of course connect to a remote server instance. > > What I meant to say is that, AFAIU, Try_Userpass uses a separate set of auth > > credentials, so that one can only set skip_validation by going through the > > ews-app. > > > > If that's not the case, do you have a suggestion for good way to skip patch validation for testing setups? > My comment was only about the Note in ChangeLog, the logic looks fine. OK great. Hopefully reads better in the updated patch.
Committed r269391: <https://trac.webkit.org/changeset/269391> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413167 [details].
<rdar://problem/71051939>
would be nice to add a unit-test as well for this.