-
Notifications
You must be signed in to change notification settings - Fork 174
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
[SCM-914] Introduce properly typed last modified date #193
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use rather Instant
. Client code needs to convert to an offset or timezone if required. We should store it neutral.
...-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/info/JGitInfoCommand.java
Outdated
Show resolved
Hide resolved
That would have the disadvantage that the original offset information gets lost. Although for most operations the normalized Instant is enough there might be edge cases where the offset might be interesting. Both Git and Subversion keep the offset as well (and don’t just emit UTC dates) |
Subversion does not. It stores Zulu in |
ca278a9
to
16b83d0
Compare
...texe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/info/GitInfoCommand.java
Show resolved
Hide resolved
16b83d0
to
a7cf6e9
Compare
maven-scm-api/src/main/java/org/apache/maven/scm/command/info/InfoItem.java
Outdated
Show resolved
Hide resolved
maven-scm-api/src/main/java/org/apache/maven/scm/command/info/InfoItem.java
Show resolved
Hide resolved
...texe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/info/GitInfoCommand.java
Outdated
Show resolved
Hide resolved
...exe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/info/GitInfoConsumer.java
Show resolved
Hide resolved
6ba25bc
to
27d69fe
Compare
0081827
to
9b182ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you double check the failing CI tests?
It only fails on Windows. I don’t have a VM or computer at hand and the logs from CI are not verbose enough for me. @michael-o Do you have a Windows machine where you could check? |
Yes, I will run it locally. |
I can reproduce the error locally. Let me check... |
Output:
|
WTF? |
Found it: The |
...-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/info/JGitInfoCommand.java
Show resolved
Hide resolved
This PR triggers apache/mina-sshd#282. We first need to upgrade MINA. I am working on this right now. |
How did this PR trigger this? |
Not intentionally, I didn't notice it before. As it seems it is also present on master. |
Populate it with svnexe, gitexe and JGit providers
914dd6f
to
4343fb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As soon as CI passes this is good to merge.
Populate it with SVN, Git and JGit for now