Skip to content

Conversation

wolfogre
Copy link
Member

I don't see why we have to use two versions of yaml. The difference between the two versions has nothing to do with our usage.

@wolfogre wolfogre changed the title Relace yaml.v2 with yaml.v3 Replace yaml.v2 with yaml.v3 Nov 16, 2022
@wolfogre wolfogre added skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR topic/distribution This PR changes something about the packaging of Gitea labels Nov 16, 2022
@wxiaoguang
Copy link
Contributor

Related:

@wolfogre wolfogre removed the topic/distribution This PR changes something about the packaging of Gitea label Nov 16, 2022
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 16, 2022
@6543
Copy link
Member

6543 commented Nov 16, 2022

that wont work as long as goldmark do use the old one

@wolfogre
Copy link
Member Author

Related:

I might know why, v2 will decode a map into interface with type map[interface{}]interface{}, but v3 will do that with map[string]interface{}, that's what I got bitten in #21831.

Not really sure, I'll take some time to figure it out.

@wolfogre
Copy link
Member Author

that wont work as long as goldmark do use the old one

I saw your attempt in yuin/goldmark-meta#6.

But I mean avoid using both versions in Gitea directly, I know we can't get rid of v2 version completely.

@lunny
Copy link
Member

lunny commented Nov 16, 2022

But at least, we can remove all direct dependencies on v2.

@wolfogre

This comment was marked as outdated.

Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

LGTM

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 21, 2022
@wolfogre
Copy link
Member Author

There are two little differences between yaml.v2 and yaml.v3 which broke the tests:

  • The v2 decodes a map into interface with type map[interface{}]interface{}, but v3 does that with map[string]interface{};
  • The v2 decodes a string like "2022-11-21T15:32:05+08:00" into interface with type string, but v3 does that with time.Time;

There's an example: https://go.dev/play/p/PqLD6tR75xF .

@wolfogre wolfogre removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Nov 21, 2022
@lunny lunny added this to the 1.19.0 milestone Nov 21, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 21, 2022
@lunny lunny merged commit e4eaa68 into go-gitea:main Nov 21, 2022
fsologureng pushed a commit to fsologureng/gitea that referenced this pull request Nov 22, 2022
I don't see why we have to use two versions of yaml. The difference
between the two versions has nothing to do with our usage.
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 23, 2022
* giteaofficial/main:
  Handle empty author names (go-gitea#21902)
  Move all remaining colors into CSS variables (go-gitea#21903)
  Add option to enable CAPTCHA validation for login (go-gitea#21638)
  Prepend refs/heads/ to issue template refs (go-gitea#20461)
  Fixes go-gitea#21895: standardize UTC tz for util tests (go-gitea#21897)
  Clarify logging documentation (go-gitea#21665)
  Update JS dependencies (go-gitea#21881)
  Webhook list enhancements (go-gitea#21893)
  Embed Matrix icon as SVG (go-gitea#21890)
  fix(web): add `alt` for logo in home page (go-gitea#21887)
  Improvements for Content Copy (go-gitea#21842)
  Replace yaml.v2 with yaml.v3 (go-gitea#21832)
  Allow disable RSS/Atom feed (go-gitea#21622)
  Consolidate security-check into checks-backend (go-gitea#21882)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants