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

Bump QD to 3 and some minor style fixes #19

Merged
merged 5 commits into from Jul 2, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jun 25, 2020

  • Bump QD to 3
  • some minor style fixes

@brawner @Blast545

Signed-off-by: ahcorde ahcorde@gmail.com

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde mentioned this pull request Jun 25, 2020
20 tasks
Copy link
Contributor

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

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

Minor comment, otherwise LGTM

QUALITY_DECLARATION.md Outdated Show resolved Hide resolved
@@ -10,9 +10,9 @@ The `libyaml` meets the basic requirements for a software platform in terms of t

Even if the library does not provide an API/ABI policy targeting the desired use of the library, the fact that it deals with the YAML standard and this one hasn’t changed since 2009, allows us to infer that the functionality needed for the ROS core from this library is not going to be changed.

There is no explicit support for any OS platform, however their [Github repository](https://github.com/yaml/libyaml) installation appears to be targeting Linux. The first version of this library was developed in 2006, and it is used widely. There is no explicit metric of how much the library is used, but the equivalent library for Python, developed by the same organization is required for at least 150k repositories (According to [Github metrics](https://github.com/yaml/pyyaml/network/dependents?package_id=UGFja2FnZS01MjUyMjEzNQ%3D%3D)) and the `libyaml` library is used for some optional fast functionality. The [safe_yaml](https://rubygems.org/gems/safe_yaml) ruby gem has over 80million downloads and one of its implementations uses `libyaml` through psych. It is also used in the [Go-yaml project](https://github.com/go-yaml/yaml), the project supporting Yaml in the Go language.
There is no explicit support for any OS platform, however their [Github repository](https://github.com/yaml/libyaml) installation appears to be targeting Linux. The first version of this library was developed in 2006, and it is used widely. There is no explicit metric of how much the library is used, but the equivalent library for Python, developed by the same organization is required for at least 150k repositories (According to [Github metrics](https://github.com/yaml/pyyaml/network/dependents?package_id=UGFja2FnZS01MjUyMjEzNQ%3D%3D)) and the `libyaml` library is used for some optional fast functionality. The [safe_yaml](https://rubygems.org/gems/safe_yaml) ruby gem has over 80million downloads and one of its implementations uses `libyaml` through psych. It is also used in the [Go-yaml project](https://github.com/go-yaml/yaml), the project supporting YAML in the Go language.
Copy link

Choose a reason for hiding this comment

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

While you're editing this paragraph, could you switch it to use one sentence per line so that new PRs can have easier to read diffs? I honestly can't tell what changed with a quick glance.

Copy link
Contributor

Choose a reason for hiding this comment

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

For cases like this one in particular, I use the "show rich diff" option within Github, and that highlights the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed Yaml -> YAML

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested review from Blast545 and brawner June 26, 2020 06:49
Signed-off-by: ahcorde <ahcorde@gmail.com>
libyaml_q_declaration.md Outdated Show resolved Hide resolved
libyaml_q_declaration.md Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from brawner June 29, 2020 17:52
Copy link

@brawner brawner left a comment

Choose a reason for hiding this comment

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

Just one more grammar thing I missed.

libyaml_q_declaration.md Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from brawner June 29, 2020 18:06
@Blast545 Blast545 merged commit af843d4 into ros2:master Jul 2, 2020
ahcorde added a commit to ahcorde/libyaml_vendor that referenced this pull request Oct 29, 2020
* Bump QD to 3 and some minor fixed
* Added feedback
* Sentences on separate lines to help minimize diffs
* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>
cottsay pushed a commit that referenced this pull request Oct 29, 2020
Backport of #19 and #23

Signed-off-by: ahcorde <ahcorde@gmail.com>
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.

None yet

3 participants