Skip to content
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

Drop old deprecated options / conditionals #7332

Merged
merged 7 commits into from Mar 13, 2019
Prev

Add todo for related issue

  • Loading branch information...
codealchemy committed Mar 11, 2019
commit ea02d63d2ef505a75ec224a55a81989c95a75e2e
@@ -112,6 +112,7 @@ def check_unknown(self, target, kwargs, payload):
"""
ignore_params = set((self.get_options().ignored or {}).get(target.type_alias, ()))
unknown_args = {arg: value for arg, value in kwargs.items() if arg not in ignore_params}
# TODO(#7357) Remove these checks once test issues are resolved.
if 'sources' in payload.as_dict():

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 8, 2019

Author Contributor

I noticed this was last touched in 450de70 - it feels like we should be able to drop these checks / pops and allow an error to be raised if they're included (or move them to ignored_args?), @illicitonion @cosmicexplorer might ya'll know on if this can be removed, moved to ignored_args, or should be left as-is?

This comment has been minimized.

Copy link
@illicitonion

illicitonion Mar 8, 2019

Contributor

Yeah, I think you should be able to remove this entire block, and just treat sources like any other arg :) Good spot!

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 11, 2019

Author Contributor

Getting rid of this caused quite a few test failures, most (maybe all? from what I've spot checked so far at least) of which are from having source= within BUILD files across pants. Would it be safe to assume those should be updated to sources=[...] accordingly, or might it be preferable to revert c09f9cb for now?

This comment has been minimized.

Copy link
@illicitonion

illicitonion Mar 11, 2019

Contributor

Oh dear - please revert that change and file a ticket assigned to me :) Sorry about that!

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 11, 2019

Author Contributor

No worries! Will revert and get a ticket open for that.

Edit: #7357

This comment has been minimized.

Copy link
@Eric-Arellano

Eric-Arellano Mar 11, 2019

Contributor

Consider adding a TODO comment linking to 7357 here.

This comment has been minimized.

Copy link
@codealchemy

codealchemy Mar 11, 2019

Author Contributor
if 'source' in unknown_args:
unknown_args.pop('source')
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.