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

Formalize the "extension" entity to replace "extensions" argument to get #404

Closed
yarikoptic opened this issue Feb 27, 2019 · 3 comments · Fixed by #432
Closed

Formalize the "extension" entity to replace "extensions" argument to get #404

yarikoptic opened this issue Feb 27, 2019 · 3 comments · Fixed by #432

Comments

@yarikoptic
Copy link
Collaborator

As I have just discovered for myself from #398, there is an extensions argument to get. But since we have already suffix, it makes only sense to add extension entity to be parsed out from the filename. Then get (and whatever else operating on entities) would be able to operate on the extension entity (and multiple would be allowed to be provided in a list as with any entity ATM AFAIK) and there would be no need for custom extensions argument to get. Am I correct or missing something?

@tyarkoni
Copy link
Collaborator

I'm pretty sure there was a rationale for handling it this way in grabbit, but I don't remember what it was. I agree that given the current codebase, and the separation from grabbit, there doesn't appear to be any current reason not to treat extension like a first-class entity. Let's leave this open until I have a chance to do a full sweep of the codebase and make sure there isn't anything going on that's not coming to mind right away.

@tyarkoni
Copy link
Collaborator

One general comment on some of your recent PRs, @yarikoptic: I agree with most of them (pending careful review) from a design standpoint. But they almost all involve major API-breaking changes even worse than those seen in 0.7, and I'm not so enthusiastic about breaking things again so soon—especially as once we start revisiting this kind of thing, there may be other things we want to break.

Maybe we should create a branch to collect all of these changes, and target 0.9 to merge it in, instead of merging each one haphazardly into master and breaking things repeatedly for people?

@yarikoptic
Copy link
Collaborator Author

Hold on, where did I break API in any recent PR?

As for the issues I am raising - in particular with get/querying - sure thing they need more thought and should go to some dangling branch first if we magically don't find a way to since them without breaking API or at least without some deprecation shimming for a release cycle or two.

Related #406
Sorry for the flood today

@tyarkoni tyarkoni mentioned this issue Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants