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

Partial revert of 4d2c4af 'Improve template mechanism' #439

Merged
merged 1 commit into from May 9, 2017

Conversation

eparis
Copy link
Collaborator

@eparis eparis commented May 9, 2017

There were template functions which we defined and others started using.
Although we no longer want those functions, since others use them,
deleting them breaks our API. Putting those (unused) functions back.

Fixes the report in #422 (comment)

There were template functions which we defined and others started using.
Although we no longer want those functions, since others use them,
deleting them breaks our API.  Putting those (unused) functions back.
@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

@BoGeM @fabianofranz

@fabianofranz
Copy link
Contributor

LGTM, also thanks for the FIXME notes.

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

I'm going to self merge to unblock kubernetes. @BoGeM we can talk about next steps if you disagree with what I'm doing here...

@eparis eparis merged commit 347767f into spf13:master May 9, 2017
@n10v
Copy link
Collaborator

n10v commented May 9, 2017

We only should return Gt and Eq functions, if we want to return backward compatibility.
We don't need to restore all templateFuncs. appendIfNotPresent is even a private function, so other users can't use it.

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

ok, will fix, but @fabianofranz you are free!!

@fabianofranz
Copy link
Contributor

Private funcs are also important since they can be used by people providing their templates through SetHelpTemplate and SetUsageTemplate.

@n10v
Copy link
Collaborator

n10v commented May 9, 2017

If they provide custom templates, they can provide their own functions by AddTemplateFunc.

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

But we don't break existing users, if someone could be using those functions via Set*Template we should not break them. (Until we decide to go to v2).

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

Mind you I'm all for going to something we call v2. The inconsistency in handling of finding subcmds vs taking unnamed arguments has always bothered me a lot :)

@n10v
Copy link
Collaborator

n10v commented May 9, 2017

If they want to use them, they can copy these functions and them by themselves. It's no problem.
They're about ~6-10 lines.

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

agreed, technically it is easy, fabiano would have created his own functions. But the contract we make with users is not to break their software unless we move to v2. If it worked yesterday, it must work tomorrow. Even if that means we are stuck with something subpar, as long as 1 person relied on it, we must keep it until we change the major version. (given we don't even have a 1.0 I know that 'major version' thing is hard to visualize)

@n10v
Copy link
Collaborator

n10v commented May 9, 2017

So what about a writing a roadmap to release v1.0?

@n10v
Copy link
Collaborator

n10v commented May 9, 2017

And we should consider #259 issue.

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

Personally I think cobra works well enough that we could just call it 1.0 right now and start working on removing all the silly things like this PR...

@n10v
Copy link
Collaborator

n10v commented May 9, 2017

Then let's write a list of silly things, what we should remove.
We can call it v2.0 Roadmap.

@eparis
Copy link
Collaborator Author

eparis commented May 9, 2017

I can think of 2 :) What's a good place? in #259?

@n10v
Copy link
Collaborator

n10v commented May 9, 2017

I think, we should make new issue for it.

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.

None yet

3 participants