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

Fix date format constant for version check. #94

Merged
merged 2 commits into from
May 4, 2023

Conversation

minnoroth
Copy link
Contributor

Change VERSION_DATE_FORMAT constant in order to prevent version check raising an exception: java.text.ParseException: Unparseable date: "2023-02-22T13:35:15Z"

@pavelhoral
Copy link
Member

pavelhoral commented Apr 25, 2023

Were you able to find where (by which commit) this inconsistency was introduced? Btw. commit message is missing issue number.

@minnoroth
Copy link
Contributor Author

Updated the commit message with the number of the issue.

The inconsistency was first introduced by this commit:
cf19dc3

@minnoroth minnoroth removed their assignment Apr 25, 2023
@pavelhoral
Copy link
Member

Implemented changes does not feel complete - i.e. changing unit tests without checking what those tests are checking is suspicious.

By a quick search you can find that the DATE_FORMAT constant is used in VersionUtils where there are multiple patterns and logic that uses UK based date format / locale. Also please be aware that by switching to ISO date format does not mean that we can expect that that is the format used in an installation that is being upgraded.

pavelhoral
pavelhoral previously approved these changes May 2, 2023
Copy link
Member

@pavelhoral pavelhoral left a comment

Choose a reason for hiding this comment

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

LGTM after refactoring

@pavelhoral pavelhoral requested a review from karelmaxa May 2, 2023 14:40
Copy link
Member

@karelmaxa karelmaxa left a comment

Choose a reason for hiding this comment

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

LGTM.

@karelmaxa karelmaxa merged commit 669ec96 into WrenSecurity:main May 4, 2023
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

3 participants