agithub icon indicating copy to clipboard operation
agithub copied to clipboard

immutable IncompleteRequest.__getattr__

Open mattsb42 opened this issue 5 years ago • 2 comments

The main discussion of the problem is in #74, but TLDR is that IncompleteRequest.__getattr__ mutating and returning self makes a lot of use-cases needlessly complicated.

This is a breaking change.

fixes: #74

I also fixed setup.py to actually work with the tox config. It looks like that used to be there but got rolled back as part of an unrelated set of changes.

mattsb42 avatar Feb 02 '20 23:02 mattsb42

@mattsb42 Thank you for the PR!

How would you propose going about introducing this breaking change (so as to avoid folks getting the new version and it breaking existing codebases?

gene1wood avatar Feb 04 '20 16:02 gene1wood

This is a good question, and IMO really depends on your plans for the project. It looks like there's been a bunch of recent movement here, I'm guessing with Mozilla taking over the project? Maybe #39 would be an opportune time to make some significant changes?

The reason I think this is important is that the current structure makes it very difficult to pass around and use any partial constructions. You have to end up doing silly things like this[1] to avoid continuously messing up the url.

Another possible option for accomplishing this without breaking backwards compatibility would be to let the user pass an option on instantiation that would determine whether or not this should be a mutable or immutable client. That would increase the API complexity, but would let the user choose this behavior if they want it, and leave the default behavior as-is.

[1] https://github.com/mattsb42/repo-manager/blob/0bef9d6f85e67c54201f89a9a1668dafa8e3d73f/src/repo_manager/_groups/branches.py#L63-L80

mattsb42 avatar Feb 26 '20 06:02 mattsb42