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

remove layers of abstraction #64

Closed
luxzeitlos opened this issue Apr 20, 2018 · 7 comments
Closed

remove layers of abstraction #64

luxzeitlos opened this issue Apr 20, 2018 · 7 comments

Comments

@luxzeitlos
Copy link
Contributor

Currently we have ember-cli/ember-cli-app-version which uses cibernox/git-repo-version which uses rwjblue/git-repo-info. I think this is too much complexity for this problem.

Also I think more and better configuration would be nice for ember-cli-app-version.
Also it seems (#49, #50, cibernox/git-repo-version#9) that we need some configuration.
Ideally I think would be to have some kind of template string the user can provide, to specify which information in which order should be shown. At least we should have a way to display everything provided by rwjblue/git-repo-info.

My suggestion is to remove cibernox/git-repo-version and directly rely on rwjblue/git-repo-info. Then we could allow the user to provide a format function that takes the info from rwjblue/git-repo-info and have a sane default. Also we could provide the prefix, which calculation is currently the most complex thing git-repo-version/index.js does.

something like this looks like a possible solution in my opinion:

formatFunction: ({prefix, sha, authorDate}) => `${prefix}${sha ? `+${sha}` : ''} ${authorDate}`,

any thoughts?

@stefanpenner
Copy link
Collaborator

whats the downside?

@luxzeitlos
Copy link
Contributor Author

what about complexity and boilerplate code?

I've had a chat in slack with @twokul about #49 and we've encountered this confusion. But I also got confused about which repo is which far to often myself.

Also for most changes this means we have to change 2 or maybe even 3 repos. This makes contribution harder.

@stefanpenner
Copy link
Collaborator

stefanpenner commented Apr 26, 2018

@luxferresum i think you misunderstand, I agree with you. I am merely asking if you have identified any downsides?

@luxzeitlos
Copy link
Contributor Author

@stefanpenner yes, sorry, I totally misunderstood!

I only see one downside: We probably will loose maintainers for cibernox/git-repo-version, and if any other project then ember-cli-app-version uses it, they have to maintain it themselves or drop it as we do.

@stefanpenner
Copy link
Collaborator

@luxferresum i think thats fine. cc @cibernox thoughts?

@cibernox
Copy link
Contributor

sure, if you find it useful let's cut the middleman

@rwjblue
Copy link
Member

rwjblue commented Jun 20, 2018

I'm happy to add more maintainers to rwjblue/git-repo-info also. It has at least @hjdivad and myself, but can also add @stefanpenner and @cibernox if they want...

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

4 participants