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

Update yaml format in docs #1496

Merged
merged 12 commits into from
Dec 24, 2018
Merged

Update yaml format in docs #1496

merged 12 commits into from
Dec 24, 2018

Conversation

Zeragamba
Copy link
Contributor

Added a rake task to run all the yaml files through the parser then output them back in place:
bundle exec rake reformat_yaml

Annoyingly... the standard YAML dump library used by Ruby doesn't provide a lot of formatting options (pretty much just indentation). We can have a look at using another dumping library if we really want to change the format.

One big thing to note is that all comments were lost due to the parser not caring about them. We can manually go through and restore comments.

`bundle exec rake reformat_yaml`

Annoyingly... the standard YAML dump library used by Ruby doesn't provide a lot of formatting options (pretty much just indentation)
Comments were lost due to the parser not caring about them. We can manually go through and restore comments.
@Zeragamba
Copy link
Contributor Author

Zeragamba commented Dec 17, 2018

Reformatting to standardize yaml format as mentioned in #1493

@richardbulger
Copy link
Contributor

Could faker/CONTRIBUTING.md and faker/lib/locales/en/README.md be update to reflect this new standard?

@Zeragamba
Copy link
Contributor Author

should we restore the comments? or leave it as is?

@stympy
Copy link
Contributor

stympy commented Dec 24, 2018

Is it just me, or did a lot of indentation get messed up? See https://github.com/stympy/faker/blob/5c5a66191de3cb4caf713d7ca47086210f1e46d3/lib/locales/tr.yml for an example.

@stympy
Copy link
Contributor

stympy commented Dec 24, 2018

@SpyMaster356 I'm all for keeping the comments, if they were useful. I skimmed but didn't see what kinds of comments were lost. Could you point me at an example?

creatures: [ "squee", "sunner", "wahrks", "ytrams", "scarab beetles", "bahro", "karnaks", "Mangree", "Zeftyr" ]
characters: [ "The Stranger", "Atrus", "Sirrus", "Achenar", "Gehn", "Catherine", "Saavedro", "Yeesha", "Esher", "Atrius", "Ti'ana" ]
ages: [
# Myst
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stympy here's some comments that were lost

@Zeragamba
Copy link
Contributor Author

Zeragamba commented Dec 24, 2018

Is it just me, or did a lot of indentation get messed up? See https://github.com/stympy/faker/blob/5c5a66191de3cb4caf713d7ca47086210f1e46d3/lib/locales/tr.yml for an example.

Apparently that indentation is what the YAML writer generates and unfortunately the stdlib YAML parser doesn't support many formatting options (or they weren't easily found in the documentation)

@stympy
Copy link
Contributor

stympy commented Dec 24, 2018

I'd rather leave the existing YAML as-is and just change the documentation to encourage the preferred format for future YAML.

@Zeragamba
Copy link
Contributor Author

sounds good to me, let me revert the changes to the yml files. I'll keep the reformatting rake task in though

@Zeragamba
Copy link
Contributor Author

Changes made, should be good now?

@Zeragamba Zeragamba changed the title Reformat yaml Update yaml format in docs Dec 24, 2018
@stympy
Copy link
Contributor

stympy commented Dec 24, 2018

Looking good, thanks!

@stympy stympy merged commit e8528ad into faker-ruby:master Dec 24, 2018
@Zeragamba Zeragamba deleted the reformat-yaml branch April 7, 2019 04:33
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants