Skip to content

Update to_s and inspect to have similar signatures accross the stdlib#7528

Merged
bcardiff merged 25 commits intocrystal-lang:masterfrom
jkthorne:unify-to_s-signature
Mar 14, 2019
Merged

Update to_s and inspect to have similar signatures accross the stdlib#7528
bcardiff merged 25 commits intocrystal-lang:masterfrom
jkthorne:unify-to_s-signature

Conversation

@jkthorne
Copy link
Contributor

@jkthorne jkthorne commented Mar 8, 2019

I noticed some differences the signatures of some to_s methods that take IO. This just adds some types explicitly and changes some of the methods to use the same signature.

ghost
ghost reviewed Mar 8, 2019
View reviewed changes
Sija
Sija reviewed Mar 8, 2019
View reviewed changes
Sija and others added 2 commits March 9, 2019 23:15
@Sija @jkthorne
Update src/big/big_rational.cr
3caccb8 
Co-Authored-By: wontruefree <javajax@gmail.com>
@Sija @jkthorne
Update src/big/big_rational.cr
84dbf95 
Co-Authored-By: wontruefree <javajax@gmail.com>
@jkthorne
Copy link
Contributor Author

jkthorne commented Mar 11, 2019

@Sija and @r00ster91 I have updated the PR what do you think?

@Sija
Copy link
Contributor

Sija commented Mar 11, 2019

👍 from me

ghost
ghost approved these changes Mar 11, 2019
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

👍 from me too.

j8r
j8r reviewed Mar 11, 2019
View reviewed changes
Copy link
Contributor

@j8r j8r left a comment

Choose a reason for hiding this comment

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

Since we are annotating, even this are compiler's internal; there are no docs but this can help debugging.

j8r and others added 18 commits March 11, 2019 16:02
@j8r @jkthorne
Update src/compiler/crystal/tools/doc/method.cr
0ead273 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
c8183fd 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
b35a7f9 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
3b9777b 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
19a4cc5 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
ca28c46 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
e0a1afd 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
8207b1d 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
5c0e966 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
1b99c18 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
b488f44 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
d326f45 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
9af91e1 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
9b61e3c 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
8ce367e 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
e7ebc8d 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
fb3f07d 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
126e653 
Co-Authored-By: wontruefree <javajax@gmail.com>
j8r and others added 2 commits March 11, 2019 16:09
@j8r @jkthorne
Update src/compiler/crystal/types.cr
9dfff06 
Co-Authored-By: wontruefree <javajax@gmail.com>
@j8r @jkthorne
Update src/compiler/crystal/types.cr
479c8f0 
Co-Authored-By: wontruefree <javajax@gmail.com>
@jkthorne
Copy link
Contributor Author

jkthorne commented Mar 11, 2019

@j8r I updated everything with your suggestions. Can you take another look?

@straight-shoota straight-shoota mentioned this pull request Mar 12, 2019
Closed
@jkthorne
Copy link
Contributor Author

jkthorne commented Mar 12, 2019

@j8r can you rerun CI for this it looks like the build was canceled?

@straight-shoota
Copy link
Member

straight-shoota commented Mar 12, 2019

I cancelled it because there way too many builds in the queue. I've just triggered a rerun.

@jkthorne
Copy link
Contributor Author

jkthorne commented Mar 14, 2019

@bcardiff I just wanted to check in and see if this could get merged?

@bcardiff bcardiff merged commit ae21e16 into crystal-lang:master Mar 14, 2019
@straight-shoota straight-shoota added this to the 0.28.0 milestone Mar 14, 2019
@bcardiff
Copy link
Member

bcardiff commented Mar 14, 2019

Now that there were two reviews yes. Thanks @wontruefree!

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)
@jkthorne jkthorne deleted the unify-to_s-signature branch July 11, 2020 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asterite asterite asterite approved these changes

@bcardiff bcardiff bcardiff approved these changes

+3 more reviewers

@Sija Sija Sija approved these changes

@j8r j8r j8r approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

kind:refactor topic:stdlib

Projects

None yet

Milestone

0.28.0

Development

Successfully merging this pull request may close these issues.

6 participants

@jkthorne @Sija @straight-shoota @bcardiff @asterite @j8r