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

Mass electron volt #275

Merged
merged 2 commits into from
Jul 11, 2017
Merged

Mass electron volt #275

merged 2 commits into from
Jul 11, 2017

Conversation

cquiroz
Copy link
Collaborator

@cquiroz cquiroz commented Jun 24, 2017

This PR adds electron volt as a Mass unit.

A possible problem here is that there are ElectronVolt units for different quantities and there could be some conflicts if you were going to import both Energy and Mass. They are different units but use the same name in the common usage

@cquiroz cquiroz changed the title Added electron-volt as a unit of mass Mass electron volt Jun 24, 2017
@derekmorr derekmorr modified the milestone: 1.4 Jun 25, 2017
@cquiroz cquiroz force-pushed the mass-electron-volt branch from 1d4f113 to b3f1904 Compare June 26, 2017 04:55
@@ -171,6 +182,45 @@ object SolarMasses extends MassUnit {
val symbol = "M☉"
}

object ElectronVolt extends MassUnit {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't this cause name collisions in the REPL since we import all constants and implicits by default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is problematic in general. Having the same name will create problems either on the REPL or other cases. I'm kind of leaning to use ElectronVolt for Energy and use something like ElectronVoltMass in this case and similar

What do you think?

@cquiroz cquiroz force-pushed the mass-electron-volt branch 2 times, most recently from 90ba512 to e6a9cd9 Compare June 28, 2017 22:22
@cquiroz cquiroz force-pushed the mass-electron-volt branch from e6a9cd9 to 823d677 Compare July 11, 2017 15:46
@cquiroz
Copy link
Collaborator Author

cquiroz commented Jul 11, 2017

I renamed the unit to ElectronVoltMass. As much as I like the original unit, it is bound to create issues to have the same name for different dimensions

@derekmorr derekmorr merged commit cad949d into typelevel:master Jul 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants