Skip to content

Fix metadata link key names#1896

Merged
homu merged 1 commit into
ruby:masterfrom
sonalkr132:fix-metadata-link-names
Apr 19, 2017
Merged

Fix metadata link key names#1896
homu merged 1 commit into
ruby:masterfrom
sonalkr132:fix-metadata-link-names

Conversation

@sonalkr132
Copy link
Copy Markdown
Contributor

Description:

I had used incorrect names for metadata link keys. Related: rubygems/rubygems.org#1234 (comment)

Good thing it wasn't released with 2.6.11. Is there anything I can do to make sure that my commits are in next release? I need to update guides.

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

@djberg96
Copy link
Copy Markdown
Contributor

👍

@sonalkr132 sonalkr132 force-pushed the fix-metadata-link-names branch from 0559115 to 01df511 Compare April 18, 2017 16:29
url_validation_regex = %r{\Ahttps?:\/\/([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z}
link_keys = ["home", "code", "docs", "wiki", "mail", "bugs", "changelog"]
link_keys = %w(
homepage_uri
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.

please sort these so future diffs will be cleaner

Comment thread test/rubygems/test_gem_specification.rb Outdated
@m1 = quick_gem 'm', '1' do |s|
s.files = %w[lib/code.rb]
s.metadata = { "one" => "two", "home" => "https://example.com/user/repo" }
s.metadata = { "one" => "two", "homepage_uri" => "https://example.com/user/repo" }
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.

please also add a test that the un-prefixed keys aren't validated?

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.

I understand the need for test, as in if correct keys were not used links won't show up on rubygems.org.
It's hard to imagine such test. un-prefixed keys won't be invalid as metadata accepts any value for its keys.

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.

we want to make sure that specifying a non-URI string for home is allowed

@sonalkr132 sonalkr132 force-pushed the fix-metadata-link-names branch from 01df511 to 6ad3087 Compare April 18, 2017 20:12
@sonalkr132 sonalkr132 force-pushed the fix-metadata-link-names branch from 6ad3087 to a86da1d Compare April 19, 2017 16:15
@sonalkr132
Copy link
Copy Markdown
Contributor Author

Updated.

@segiddins
Copy link
Copy Markdown
Contributor

Wonderful, thank you so much @sonalkr132 !

@homu r+

@homu
Copy link
Copy Markdown
Contributor

homu commented Apr 19, 2017

📌 Commit a86da1d has been approved by segiddins

@homu
Copy link
Copy Markdown
Contributor

homu commented Apr 19, 2017

⚡ Test exempted - status

@homu homu merged commit a86da1d into ruby:master Apr 19, 2017
homu added a commit that referenced this pull request Apr 19, 2017
Fix metadata link key names

# Description:

I had used incorrect names for metadata link keys. Related: rubygems/rubygems.org#1234 (comment)

Good thing it wasn't released with 2.6.11. Is there anything I can do to make sure that my commits are in next release? I need to update guides.
# Tasks:

- [x] Describe the problem / feature
- [ ] Write tests
- [x] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).
@sonalkr132 sonalkr132 deleted the fix-metadata-link-names branch April 20, 2017 02:56
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.

5 participants