Skip to content
This repository was archived by the owner on Jul 12, 2025. It is now read-only.

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jun 9, 2018

This fixes #679. The solution is more complicated than just forbidden the method. I choose to let it work more like Ruby:

  1. Class < Module and Module < Object
  2. Module class doesn't support #new (but support .new)
  3. Class class support #new

Before today our inheritance chain for Class class is Class < Object, so I changed it as described above.

@hachi8833 can you help me compare the Goby's behavior with Ruby again?

@st0012 st0012 added this to the version 0.1.10 milestone Jun 9, 2018
@st0012 st0012 self-assigned this Jun 9, 2018
@ghost ghost added the in progress label Jun 9, 2018
@codecov
Copy link

codecov bot commented Jun 9, 2018

Codecov Report

Merging #681 into master will increase coverage by 0.05%.
The diff coverage is 94.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   81.94%   81.99%   +0.05%     
==========================================
  Files          57       57              
  Lines        7764     7798      +34     
==========================================
+ Hits         6362     6394      +32     
- Misses       1132     1133       +1     
- Partials      270      271       +1
Impacted Files Coverage Δ
vm/boolean.go 92.3% <100%> (ø) ⬆️
vm/hash.go 97.02% <100%> (ø) ⬆️
vm/http_client.go 59.63% <100%> (ø) ⬆️
vm/string.go 95.81% <100%> (ø) ⬆️
vm/range.go 89.89% <100%> (ø) ⬆️
vm/simple_server.go 68.21% <100%> (ø) ⬆️
vm/concurrent_rw_lock.go 92.4% <100%> (ø) ⬆️
vm/null.go 88.23% <100%> (ø) ⬆️
vm/http.go 87.15% <100%> (ø) ⬆️
vm/integer.go 73.91% <100%> (ø) ⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05e9fa3...92dc229. Read the comment docs.

@st0012
Copy link
Member Author

st0012 commented Jun 10, 2018

@hachi8833 can you review and approve it if looks ok?

Copy link
Member

@hachi8833 hachi8833 left a comment

Choose a reason for hiding this comment

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

Thank you for the implementation!

I checked the updated behaviors with Ruby's ones:

  • Class inherits Modules
  • Module inherits Object
  • Inheriting modules on classes: prohibited
  • inheriting classes on modules: prohibited
  • performing #new on modules: prohibited
  • performing Module.new: permitted
  • performing Class.new: permitted

I think this can be merged.

@st0012 st0012 merged commit b5f9cf4 into master Jun 10, 2018
@ghost ghost removed the in progress label Jun 10, 2018
@st0012 st0012 deleted the Prohibit-module-initialization branch June 10, 2018 06:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undocumented: Goby's modules can be new
2 participants