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

Improvement #4

Merged
merged 2 commits into from
Nov 13, 2022
Merged

Improvement #4

merged 2 commits into from
Nov 13, 2022

Conversation

vigneshwarn
Copy link

@vigneshwarn vigneshwarn commented Nov 12, 2022

  1. Added status code 429 Exception
  2. Added created/updated time in ZonedDateTime format

2. Added created/updated time in ZonedDateTime format
@vigneshwarn vigneshwarn changed the title 1. Added status code 429 Exception Improvement Nov 12, 2022
@shivam1608
Copy link
Owner

👋 thank you for showing interest in this project.
but do we really need ZonedDateTime ? i don't think so because the developer using the library can easily parse that thing i didn't find it useful
however your update 1.Added status code 429 Exception is reasonable so you have to remove the ZonedDateTime so i can merge the request also kindly revert the changes in pom.xml regarding version because it gets changed in major updates
if you wish you can update the documentation too.

@vigneshwarn
Copy link
Author

@shivam1608 I have seen most of the developers were using ZonedDateTime. Which will be very useful for comparison/verification/Time-related wait etc......, I was using your Lib for a very long back thought this thing is missing, what do u feel. Also the String createdAt & String updatedAt are still available. After ur confirmation i can will update the PR

Copy link
Owner

@shivam1608 shivam1608 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after thinking the use case of ZonedDateTime i think we should add it to the library so kindly fix these issue to continue merge

src/main/java/me/shivzee/util/Utility.java Outdated Show resolved Hide resolved
src/main/java/me/shivzee/util/Message.java Outdated Show resolved Hide resolved
src/main/java/me/shivzee/util/Message.java Outdated Show resolved Hide resolved
@shivam1608 shivam1608 added the enhancement New feature or request label Nov 13, 2022
@shivam1608 shivam1608 merged commit cad6af6 into shivam1608:main Nov 13, 2022
@shivam1608
Copy link
Owner

nice work 👍will be usable in the next release
till then you can use main-SNAPSHOT as version if you want to use before release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants