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.

Sign up for GitHub

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

Jump to bottom

YAML: fix test checking serialization of slices for libyaml 0.2.2 #7555

Merged
merged 1 commit into from
Mar 15, 2019
Merged

YAML: fix test checking serialization of slices for libyaml 0.2.2 #7555

merged 1 commit into from
Mar 15, 2019

Conversation

asterite
Copy link
Member

Fixes #7548

I'm marking this as a (small) breaking change because I changed YAML.libyaml_version to return a SemanticVersion object, which is easier to deal with.

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. breaking-change kind:specs topic:stdlib:serialization labels Mar 15, 2019
@asterite asterite force-pushed the bug/fix-yaml-update branch from f19639d to 52a5414 Compare March 15, 2019 16:51
bcardiff
bcardiff approved these changes Mar 15, 2019
View reviewed changes
Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

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

Yup that would do it of course :-)

I think we are going to deep in the specs. We should only care about write/reading consistent values in yaml rather than checking exactly the generated file. That would lead in lest fragile specs.

@asterite
Copy link
Member Author

@bcardiff You are right. I considered testing exp == YAML.parse(YAML.dump(exp)). However, that would be testing two things at the same time. Maybe it's not that bad. But also this PR wasn't that hard to do and it's less obtrusive.

@asterite asterite merged commit d0dc06b into crystal-lang:master Mar 15, 2019
@asterite asterite added this to the 0.28.0 milestone Mar 15, 2019
@asterite
Copy link
Member Author

Merged more or less quickly because all PRs are failing in darwin because of this.

urde-graven pushed a commit to urde-graven/crystal that referenced this pull request Mar 20, 2019
Merge branch 'master' of github.com:crystal-lang/crystal
cae4845 
* 'master' of github.com:crystal-lang/crystal:
  Change the font-weight used in Playground (crystal-lang#7552)
  Fix formatting absolute paths (crystal-lang#7560)
  Refactor IO::Syscall as IO::Evented (crystal-lang#7505)
  YAML: fix test checking serialization of slices for libyaml 0.2.2 (crystal-lang#7555)
  Let Array#sort only use `<=>`, and let `<=>` return `nil` for partial comparability (crystal-lang#6611)
  Update `to_s` and `inspect` to have similar signatures accross the stdlib (crystal-lang#7528)
  Fix restriction of valid type vs free vars (crystal-lang#7536)
  Rewrite macro spec without executing a shell command (crystal-lang#6962)
  Suggest `next` when trying to break from captured block  (crystal-lang#7406)
  Fix GenericClassType vs GenericClassInstanceType restrictions (crystal-lang#7537)
  Add human readable formatting for numbers (crystal-lang#6314)
  Add command and args to execvp error message (crystal-lang#7511)
  Implement Set#add? method (crystal-lang#7495)
@asterite asterite deleted the bug/fix-yaml-update branch March 30, 2019 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcardiff bcardiff bcardiff approved these changes

@ysbaddaden ysbaddaden ysbaddaden approved these changes

Assignees
No one assigned
Labels
breaking-change kind:bug A bug in the code. Does not apply to documentation, specs, etc. kind:specs topic:stdlib:serialization
Projects
None yet
Milestone
0.28.0
Development

Successfully merging this pull request may close these issues.
3 participants
@asterite @ysbaddaden @bcardiff