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

5.0 incompatible changes plan #435

Open
5 of 11 tasks
cben opened this issue Mar 10, 2020 · 10 comments
Open
5 of 11 tasks

5.0 incompatible changes plan #435

cben opened this issue Mar 10, 2020 · 10 comments

Comments

@cben
Copy link
Collaborator

cben commented Mar 10, 2020

@cben
Copy link
Collaborator Author

cben commented Mar 10, 2020

  • What about Remove deprecated KubeException [WIP] Remove deprecated KubeException #293?
    => Not now. As explained there, HttpError is not sufficient, we need some top-level exception. Should probably introduce a Kubeclient::Error but that's a compatible change, and should give users time to adopt it before dropping KubeException.

@cben
Copy link
Collaborator Author

cben commented Apr 7, 2020

Apologies for delays. Working from home with kids has been... challenging.
I hope to get a lot done next week.

@cben cben mentioned this issue Jun 14, 2020
@cben cben changed the title 5.0 imcompatible changes plan 5.0 incompatible changes plan Jun 14, 2020
@cben cben mentioned this issue Jul 3, 2020
@cben cben pinned this issue Sep 9, 2020
@grosser
Copy link
Contributor

grosser commented Oct 11, 2020

I'd propose:

  • get rid of most of the api:
    User has to set the api version and resource anyway, so much more efficient and simple to call client.get("nodes") without doing the extra api discovery 🤷
  • remove struct and return hashes ... it's an expensive crutch :D

@cben
Copy link
Collaborator Author

cben commented Nov 3, 2020

get rid of most of the api

That will be an entirely different gem 😄
There is a non-breaking road though: first implement #332, then much later see how well people adopted new style and how people feel about dropping existing API.

remove struct and return hashes

Now that it's simple matter of ``as: :parsed/as: :parsed_symbolized`, I don't see a large enough payoff to justify breaking compatibility.
We could do small nudges like make `as:` required in `Client` contructor to stop encouraging a bad choice. Or even make people require and inject recursive-open-struct and stop depending on it?
But forcing people with lots of working `.foo.bar` code to rewrite everything to `[:foo][:bar]` form would be a disservice IMHO.

If you care about this, start by changing README to prominently explain why the new modes are recommended.

In short, both of these points sound more "8.0" (and maybe never) then "5.0" to me :)

@cben
Copy link
Collaborator Author

cben commented Nov 3, 2020

@grosser
Copy link
Contributor

grosser commented Nov 3, 2020 via email

@grosser
Copy link
Contributor

grosser commented Nov 4, 2020

stumbled into the whole url parsing mess again 😞
so add to the list:

  • make version required
  • make version apps/v1 instead of mashing up host/apis/apps and v1
  • dynamically set api or apis depending on version == v1
    ... and remove all the old glue/parsing code to have a single interface

@grosser
Copy link
Contributor

grosser commented Nov 16, 2020

plz start a v5 branch so I can make PRs against that

@cben
Copy link
Collaborator Author

cben commented Nov 16, 2020

master is already v5, recent releases were from backports to v4.y branch

@cben
Copy link
Collaborator Author

cben commented May 5, 2021

Paternity leave ended 👶, sleeplessness continues, I didn't make progress I hoped for :-(

  • If someone wants to help, one simple but important step is update changelog with everything unreleased we've merged on master.

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

No branches or pull requests

2 participants