Skip to content

fix(jsonpath): Prevent converting date string to DateTime in JSONPath#5130

Merged
niheaven merged 3 commits intoScoopInstaller:developfrom
shiena:fix/jsonpath-with-datetime
Sep 9, 2022
Merged

fix(jsonpath): Prevent converting date string to DateTime in JSONPath#5130
niheaven merged 3 commits intoScoopInstaller:developfrom
shiena:fix/jsonpath-with-datetime

Conversation

@shiena
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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