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

Rename Mass.js to ISLCMass.js #31

Closed
jessegreenberg opened this issue Feb 28, 2018 · 6 comments
Closed

Rename Mass.js to ISLCMass.js #31

jessegreenberg opened this issue Feb 28, 2018 · 6 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

Considering the naming of types in this repo and the fact that there are other "Mass" types for other sims in the project, I might recommend renaming Mass to ISLCMass. Thoughts @mbarlow12?

@jessegreenberg
Copy link
Contributor Author

For #30

@mbarlow12
Copy link
Contributor

I don't think I agree, and I'm not sure that it belongs in the common repo. I'd vote to either 1) move it to both GFL & GFLB or 2) move it to GFL and inherit it in GFLB. They both have some trade offs, but I think it's a cleaner code organization/hierarchy.

@jessegreenberg
Copy link
Contributor Author

They both have some trade offs, but I think it's a cleaner code organization/hierarchy.

Can you elaborate? What is the benefit of having GFLB dependent on GFL? At the moment all three sims are only dependent on inverse-square-law-common.

@jessegreenberg
Copy link
Contributor Author

Ah, maybe you were thinking types in ISLC should be agnostic of mass or charge?

@jessegreenberg
Copy link
Contributor Author

If that is the case I do prefer option 2 of #31 (comment) to avoid duplicated code. @mbarlow12 would you mind working on this transition if you think it is best? gravity-force-lab will need to be added as a dependency to gravity-force-lab-basics in package.json.

@mbarlow12
Copy link
Contributor

@jessegreenberg sure thing.

mbarlow12 added a commit to phetsims/gravity-force-lab that referenced this issue Mar 1, 2018
mbarlow12 added a commit that referenced this issue Mar 1, 2018
mbarlow12 added a commit to phetsims/gravity-force-lab-basics that referenced this issue Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants