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

Introduce possibility to use different renderers #67

Merged
merged 2 commits into from
Jul 9, 2015
Merged

Introduce possibility to use different renderers #67

merged 2 commits into from
Jul 9, 2015

Conversation

pdalpra
Copy link
Member

@pdalpra pdalpra commented Jul 6, 2015

Currently, sbt-buildinfo only allows to write the build info in the form of a Scala class.
It could, however, be interesting to allow to dump the build in another format, e.g. JSON.
This PR introduces BuildInfoRenderer (along with a few other data types), a trait which allows just that.

The current logic for the creation of the Scala class is moved to a dedicated class, ScalaClassRenderer and retrofitted to implement BuildInfoRenderer, set as the default renderer.

Existing users of sbt-buildinfo won't be affected by this PR, as the existing model or no pre existing key, only a new key is introduced to specify the renderer to use.

@eed3si9n
Copy link
Member

eed3si9n commented Jul 9, 2015

This looks like a pretty big change. I'm ok with it, but can I make you a committer in case we have to maintain it going forward?

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

It mostly refactoring but it does introduce a few new mechanisms indeed.
And yes, go ahead, you can make me a committer :)

eed3si9n added a commit that referenced this pull request Jul 9, 2015
Introduce possibility to use different renderers
@eed3si9n eed3si9n merged commit c180e21 into sbt:master Jul 9, 2015
@eed3si9n
Copy link
Member

eed3si9n commented Jul 9, 2015

Merged, and welcome to the team :)

@pdalpra pdalpra deleted the renderer branch July 9, 2015 12:08
@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

Thanks :)

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

BTW, when you're going to cut a new release, do not hesitate to ping me here or on Twitter so that I can update the README to document that feature ;)

@eed3si9n
Copy link
Member

eed3si9n commented Jul 9, 2015

I was gonna ask if you want to hand merge all the PRs you just broke :)

@eed3si9n
Copy link
Member

eed3si9n commented Jul 9, 2015

I'm currently looking at #58

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

I'll take #61 !

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

Shall I merge them all ?

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

Working on #65 now

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.

2 participants