Skip to content

Conversation

@tbabej
Copy link

@tbabej tbabej commented Oct 21, 2014

Adds a new configuration option, "oid-git", which specifies the
directory containing a git repository that serves as database
for OIDs.

The pushpatches.py tool has been extended to check for OID collision
and unregistered OIDs.

Adds a new configuration option, "oid-git", which specifies the
directory containing a git repository that serves as database
for OIDs.

The pushpatches.py tool has been extended to check for OID collision
and unregistered OIDs.
@encukou
Copy link
Owner

encukou commented Oct 21, 2014

Are you sure this is the right place for such a check? Shouldn't this be in
a pre-commit hook, or in CI?

On Tue, Oct 21, 2014 at 3:12 PM, Tomas Babej notifications@github.com
wrote:

Adds a new configuration option, "oid-git", which specifies the
directory containing a git repository that serves as database
for OIDs.

The pushpatches.py tool has been extended to check for OID collision

and unregistered OIDs.

You can merge this Pull Request by running

git pull https://github.com/tbabej/ipa-tools master

Or view, comment on, or merge it at:

#2
Commit Summary

  • Add automated checking for OID collision and unregistered OIDs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#2.

@tbabej
Copy link
Author

tbabej commented Oct 21, 2014

With pre-commit hooks you rely on the patch author to have the check in place. As for the CI, it can find the issue, but only after the push has been done.

If you really want to be sure you're not pushing unregistered OIDs into the main tree, then the check should be part of the tool that does the pusing.

@encukou
Copy link
Owner

encukou commented Oct 21, 2014

But the patch author and reviewer should really be the ones checking for
this kind of stuff.

By this logic we could as well run the tests before pushing. (Not that we
shouldn't, but adding ad-hoc checks to the pushing tool isn't the way to
go.)

And anyway, ipatool automates a less-than-perfect a flawed process; there's
faint hope that the process improves and ipatool becomes irrelevant. It
shouldn't take tests/checks with it.

It would be much better to make a script that can be used as a git hook
(and then possibly also call it from ipatool push).

On Tue, Oct 21, 2014 at 3:30 PM, Tomas Babej notifications@github.com
wrote:

With pre-commit hooks you rely on the patch author to have the check in
place. As for the CI, it can find the issue, but only after the push has
been done.

If you really want to be sure you're not pushing unregistered OIDs into
the main tree, then the check should be part of the tool that does the
pusing.


Reply to this email directly or view it on GitHub
#2 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants