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(jsonpath): Prevent converting date string to DateTime in JSONPath #5130

Merged
merged 3 commits into from
Sep 9, 2022

Conversation

shiena
Copy link
Contributor

@shiena shiena commented Aug 30, 2022

Description

If you extract the date and time string with checkver/jsonpath in the manifest file, it will be converted to M/d/yyyy h:m:s tt format instead of the original string. This is due to Json.net converting to DateTime type, so it suppresses the conversion.

Motivation and Context

Closes #5129

How Has This Been Tested?

  1. scoop bucket add version
  2. modify goneovim-nightly.json with goneovim-nightly: Fix version Versions#618
  3. modify lib/json.ps1 with this PR
  4. checkver.ps1 goneovim-nightly ~/scoop/bucket/versions/bucket -f
  5. update successful

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@rashil2000
Copy link
Member

I'm okay with this fix, but how do we ensure it doesn't break any existing manifest's checkver (because the checkver output will now be different)?

/cc @niheaven

@niheaven
Copy link
Member

niheaven commented Sep 1, 2022

Will review it soon, should test as many as we can to ensure the change of parser type doesn't cause problems.

@niheaven niheaven changed the title fix: Prevent converting date string to DateTime in jsonpath fix(jsonpath): Prevent converting date string to DateTime in JSONPath Sep 9, 2022
@niheaven niheaven merged commit 5ad35d6 into ScoopInstaller:develop Sep 9, 2022
@shiena shiena deleted the fix/jsonpath-with-datetime branch September 9, 2022 05:49
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.

3 participants