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

Added a ByteSizeConverter #169

Merged
merged 5 commits into from
Jun 15, 2016
Merged

Conversation

StFS
Copy link
Contributor

@StFS StFS commented May 11, 2016

Here is a suggestion for a ByteSizeConverter for what was requested in #155.

I've added this to the owner-extras module as this is not a part of core functionality.

The code should be Java5 compatible and includes a unit test.

I also did a little refactoring by introducing a ConverterUtil class to avoid duplicating code in the DurationConverter and ByteSizeConverter classes. I added that to the owner core module so that both owner-extras and owner-java8 modules would have access to it. That code is Java5 compliant so that should be fine.

@StFS
Copy link
Contributor Author

StFS commented May 11, 2016

@lviggiano I know about the lack of documentation for both this and DurationConverter. I will try to find some time to document these features this week.

@StFS
Copy link
Contributor Author

StFS commented May 11, 2016

After merging this, the only thing missing from what I was requesting in #155 is the actual ability to register converters as "default converters". That is, being able to add DurationConverter as a converter that would be used for config interface methods that return java.time.Duration by default instead of having to annotate these methods with the ConverterClass annotation explicitly (and the same then for other converters such as the ByteSizeConverter and thereby the ByteSize return type).

I'm not quite sure how to tackle that... but then again, I haven't really given it much thought. Any pointers would be appreciated and I can try to cook something up and create a PR.

@StFS
Copy link
Contributor Author

StFS commented May 11, 2016

darn it... It seems that I'm using Java8 methods in the ByteSize classes... I'll move the code to the owner-java8 module.

@StFS
Copy link
Contributor Author

StFS commented May 12, 2016

I've added some documentation. Not quite sure if it's up to standards, all comments are welcomed.

@lviggiano
Copy link
Collaborator

I'll check it and merge this asap. Thanks

@lviggiano
Copy link
Collaborator

Hi.

I am merging right now.

I moved the ByteSize classes into owner-extras, since they are not java8 related.

I left the Duration classes into owner-java8, but I think I should create a new module (owner-java8-extras), since the owner-java8 was supposed to contain just the owner core functionalities that are enabled by the use of java 8. The DurationConverter is not a core feature, but a utility. For now I keep it there in owner-java8 module and i'll decide later.

Regards
L.

@lviggiano
Copy link
Collaborator

I just discovered that also ByteSize does uses java8 APIs.

So I think I'll move your changes into a owner-java8-extras module before merging to master, next days, before merging to master.

Thanks.

L.

@lviggiano lviggiano merged commit 10881bd into matteobaccan:master Jun 15, 2016
@lviggiano
Copy link
Collaborator

lviggiano commented Jun 15, 2016

I merged it.

Take in mind that now you need to declare owner-java8-extras dependency (which also depends on owner-java8) to use the DurationConverter and the ByteSizeConverter.

Thanks again.
L.

@StFS
Copy link
Contributor Author

StFS commented Jun 15, 2016

Sure thing... btw, do you have any ETA on an official release where this will be included?

@lviggiano
Copy link
Collaborator

The minimum to release is to document all the chsnges made since last
release. If you want to help I can give you some hints on how to run the
website locally to verify the changes
Il 15 giu 2016 12:53, "Stefán Freyr Stefánsson" notifications@github.com
ha scritto:

Sure thing... btw, do you have any ETA on an official release where this
will be included?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#169 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAJ0oBcdpRSkCkdDlrRwr6-Wz0rMTcaJks5qL9mpgaJpZM4IcM9v
.

@StFS
Copy link
Contributor Author

StFS commented Jun 15, 2016

I added documentation for the duration and byte size stuff:
f4887b4

Is this not sufficient? Can you give me some hints on what you think is missing?

edit or are you talking about other features that have been added? Is there a list somewhere or do I need to do a diff in git from some release tag?

@StFS
Copy link
Contributor Author

StFS commented Jun 15, 2016

Would you say that this is an accurate view of the differences since the last release:
owner-parent-1.0.9...master

...and if so, what does it look like still needs documentation?

@lviggiano
Copy link
Collaborator

I'll write a note with things that needs to be documented. It's long time
I'm not releasing; the procedure on sonatype might have changed a little
bit, to release the jars on maven central. But no worries.
Il 15 giu 2016 4:44 PM, "Stefán Freyr Stefánsson" notifications@github.com
ha scritto:

Would you say that this is an accurate view of the differences since the
last release:
owner-parent-1.0.9...master
owner-parent-1.0.9...master


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#169 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAJ0oPgnz3PKjLYcWCmPdimK4qMdZ6YAks5qMA_PgaJpZM4IcM9v
.

@StFS
Copy link
Contributor Author

StFS commented Jul 27, 2016

any news on ETA of the next release?

@StFS StFS deleted the bytesize-converter branch July 27, 2016 11:25
@StFS
Copy link
Contributor Author

StFS commented Jan 5, 2017

I'd really like to see a new version published. Is there anything I can do to help make that happen @lviggiano?

@lviggiano
Copy link
Collaborator

Hi.

I am not working for health reasons for long time on this project. I'm sorry about that.

If you (or someone else) have a proven confidence about how to release jars on maven central, I would agree to someone else having the rights to publish the new jars.
Unfortunately the deploy process, last time I did it, was a kind of nightmare, and once a jar is released it cannot be retired or fixed.

For the moment I am trying to get my hands back on coding. But I cannot make promises.

Luigi.

@StFS
Copy link
Contributor Author

StFS commented Jan 10, 2017

Sorry to hear about your health problems.

I admit that I have not released jars to maven central before so I probably would have a rough time figuring that process out.

We've been discussing this here at work and how we would hate to see this project stagnate. We do worry though that there is no other member of this project that merges pull requests. We have discussed forking the project so that we can get changes merged into "our version" but of course we really would like to avoid that. We might end up forking but keeping that version private to us but of course still making pull requests to OWNER with any changes that we make, in the hope that these changes will eventually find their way into a new release.

It goes without saying that we hold no grudge towards you and absolutely understand your situation. We sincerely thank you for your effort in creating this project and the work you've done here. We hope that you will get better and that takes precedence over all else.

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