Skip to content

Detect duplicate keys in frontmatter YAML#628

Merged
zachmargolis merged 3 commits intomainfrom
margolis-yaml-dupes
Apr 29, 2021
Merged

Detect duplicate keys in frontmatter YAML#628
zachmargolis merged 3 commits intomainfrom
margolis-yaml-dupes

Conversation

@zachmargolis
Copy link
Copy Markdown
Contributor

No description provided.

@zachmargolis zachmargolis requested a review from jmdembe April 29, 2021 17:42
duplicate_keys = []

validator = ->(node, parent_path) do
if node.is_a?(Psych::Nodes::Mapping)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add psych to Gemfile so that we're not relying on a transitive dependency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The built-in ruby parser is psych these days, so I think it's fine as-is?

irb(main):001:0> defined?(Psych)
=> nil
irb(main):002:0> require 'yaml'
=> true
irb(main):003:0> defined?(Psych)
=> "constant"

Copy link
Copy Markdown
Contributor

@aduth aduth Apr 29, 2021

Choose a reason for hiding this comment

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

The built-in ruby parser is psych these days, so I think it's fine as-is?

irb(main):001:0> defined?(Psych)
=> nil
irb(main):002:0> require 'yaml'
=> true
irb(main):003:0> defined?(Psych)
=> "constant"

Ah, I didn't know that, was primarily going by...

https://github.com/18F/identity-idp/blob/fca67c6ddb41b3768abc346eddf89314f70c8916/Gemfile#L110

Seems fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FWIW the other place we use it isn't the IDP, it's the idp-config repo which doesn't have psych in its Gemfile either

@zachmargolis zachmargolis merged commit 1f58aa8 into main Apr 29, 2021
@zachmargolis zachmargolis deleted the margolis-yaml-dupes branch April 29, 2021 18:12
aduth added a commit to 18F/identity-idp that referenced this pull request May 13, 2021
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