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

Date test fix attempt #2 #17

Closed
wants to merge 1 commit into from
Closed

Date test fix attempt #2 #17

wants to merge 1 commit into from

Conversation

tkroman
Copy link
Contributor

@tkroman tkroman commented Apr 29, 2016

Fixes #16 now

@sarmbruster sarmbruster mentioned this pull request May 2, 2016
@jexp
Copy link
Member

jexp commented May 16, 2016

I'm still scared to try it out :) could you rebase it and we'll see if travis gets green?

@tkroman
Copy link
Contributor Author

tkroman commented May 16, 2016

@jexp I accidentally committed with the incorrect accoun/emailt, so there's a weird commit history now. Considering you mostly accept this in a manual fashion, would that be a problem?

UPD. Travis seems to be OK with my changes. Yay.

@ikwattro
Copy link
Contributor

@tkroman What's the status, do you want us to merge manually ?

@tkroman
Copy link
Contributor Author

tkroman commented May 21, 2016

@ikwattro If that will make git log look prettier than it would look otherwise (which I think it will). Anyway, there might be conflicts with #43.
Most importantly, note that this PR not only brings test fixes, but also switches to a ZoneId#getDisplayName(), which should be more reliable and environment-independent than now.

@ADTC
Copy link
Contributor

ADTC commented May 21, 2016

@tkroman I looked at your change in this PR. As I didn't make any changes in the same area, I don't expect any conflicts. 😄

PS: Disagree with manual merge. It's best to preserve attribution through pull request merges. Maybe in exceptional cases, manual merge can be done providing correct author attribution. However, I recommend against it being standard practice.

@ikwattro
Copy link
Contributor

I tend to agree also. I prefer to keep history, or ask the author to squash itself.

@ikwattro
Copy link
Contributor

I'll check with the team about merging locally / squash etc. For now I followed what was done by MH until now. Thanks @ADTC and @tkroman for your contributions.

@ADTC I'm not aware about the releasing policy yet, however I can send you the jar via PM on Slack.

@ikwattro ikwattro closed this May 21, 2016
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.

None yet

4 participants