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

show half hour marks on the left-hand column #193

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

dgrant
Copy link

@dgrant dgrant commented Oct 12, 2015

Here is the "show half hours change", now rebased on develop branch.

halfhours

@@ -7,5 +7,5 @@
*/
public interface DateTimeInterpreter {
String interpretDate(Calendar date);
String interpretTime(int hour);
String interpretTime(int hour, int minutes);
Copy link
Author

Choose a reason for hiding this comment

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

Should I create a new method with interpretTime(int hour, minutes) and leave the original interpretTime(int hour) to not break people?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, preferably.

Copy link
Author

Choose a reason for hiding this comment

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

Hold on, that doesn't actually help. Just by adding the interpretTime(int hour, int minutes) to the interface will break an client implementing that interface. Only if we had a base class and people extended that, could we then add a method to the interface and not break consumer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe have a "subinterface"?

Because a breaking change for this small behaviour change is maybe not what we want.

@dgrant
Copy link
Author

dgrant commented Oct 16, 2015

Rebased changes

@dgrant
Copy link
Author

dgrant commented Nov 10, 2015

Can this be merged?

@entropitor
Copy link
Contributor

This is still a breaking change, no?

Because I'm not sure if I want to break the library for something small like this.

@marunjar
Copy link
Contributor

hi, would be great if this pr can be merged

i think this extension of the interface is a very small breaking change and can be easily fixed by anyone.

@entropitor
Copy link
Contributor

Fine for me. @alamkanak Is it ok for you as well?

@dgrant Can you rebase onto master again?

@dgrant
Copy link
Author

dgrant commented Dec 22, 2015

@caske33 Ok, I've rebased it on master.

@entropitor
Copy link
Contributor

Can you squash those commits?

Also, there still seems to be a merge conflict. Maybe try rebasing on develop?

Thank you for your patience! 💯

@dgrant
Copy link
Author

dgrant commented Jan 18, 2016

Ok, finally cleaned everything up. Sorry for the delay.

@marunjar
Copy link
Contributor

marunjar commented Feb 2, 2016

👍

@varsha1609
Copy link

varsha1609 commented Jun 15, 2016

showHalfHours not working , Need Help

@Gurpreet258
Copy link

how to use timespacing. ? showHalfHours changes the left column to show time with half hour intervals . rest all working same i.e on hour basis. Need urgent help

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.

6 participants