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

Feature: AmountOfSubstance #304

Merged
merged 8 commits into from
Nov 7, 2017
Merged

Feature: AmountOfSubstance #304

merged 8 commits into from
Nov 7, 2017

Conversation

0xferit
Copy link
Contributor

@0xferit 0xferit commented Oct 26, 2017

No description provided.

@0xferit
Copy link
Contributor Author

0xferit commented Oct 26, 2017

We built successfully in our fork, don't know why it didn't build in AppVeyor. I'm going to investigate this issue as soon as possible.

@0xferit
Copy link
Contributor Author

0xferit commented Oct 27, 2017 •
edited
Loading

Any ideas how to deal with this error?

[10/27/2017 11:49:04 AM Error] Testhost process exited with error: It was not possible to find any compatible framework version
The specified framework 'Microsoft.NETCore.App', version '1.1.2' was not found.

  • Check application dependencies and target a framework version installed at:
    \
  • Alternatively, install the framework version '1.1.2'.

Because of this I can't load all the tests. 1024 of them runs successfully though.

@0xferit 0xferit changed the title Feature: Added SubstanceAmount Quantity Type [WIP] Feature: Added SubstanceAmount Quantity Type Oct 27, 2017
@0xferit
Copy link
Contributor Author

0xferit commented Oct 27, 2017 •
edited
Loading

Installing DotNetCore.1.0.5_1.1.2-WindowsHosting solved the problem. 2035 2048 tests (I guess it's all of them) successfully run. Could you give a hand?
@angularsen @eriove I don't know what else I can do, my build and tests are successful and I followed the steps for adding a new unit in the wiki, exactly.

@0xferit
Copy link
Contributor Author

0xferit commented Oct 27, 2017

#303

@0xferit 0xferit changed the title [WIP] Feature: Added SubstanceAmount Quantity Type Feature: Added SubstanceAmount Quantity Type Oct 27, 2017
@angularsen
Copy link
Owner

Not sure why the build server doesn't like this, the changes look good to me. I'll investigate a bit.

My only feedback here is as mentioned in issue:

  1. Should we add PoundPerMole?
  2. Should it be named AmountOfSubstance or SubstanceAmount? I have a preference for AmountOfSubstance, because that is the exact wording used everywhere when I google it.

@angularsen
Copy link
Owner

angularsen commented Oct 27, 2017 •
edited
Loading

Ok, figured it out. You need to merge in latest origin/master (or rebase on top of it) to get the newest code generation scripts. Your branch is based on the old scripts and you generated a file in the wrong location, and the build server will also regenerate the code when it simulates merging into origin/master to be doubly sure, and then it gets two of the same file.

You also need to delete/move your file in the UnitClasses folder, it should sit in Quantities folder.

Make sure you merge in the master from angularsen repo, and not your own fork's master.

@0xferit
Copy link
Contributor Author

0xferit commented Nov 6, 2017 •
edited
Loading

PoundPerMole is implemented in the other pull request. We don't use it in our project at the moment but I guess it can be useful for us or someone else later on. What do you prefer? To have it or not to have it?

I'm okay with both SubstanceAmount and AmountOfSubstance. Reason I chose SubstanceAmount is source-code readability.

As soon as you let me know your decision, I'll proceed.

@0xferit
Copy link
Contributor Author

0xferit commented Nov 6, 2017

Oh @angularsen , did you mean PoundMole by saying PoundPerMole?

@angularsen
Copy link
Owner

Ah, yes PoundMole. I think it's good to add for consistency and completeness.

@angularsen
Copy link
Owner

Also, let's go with AmountOfSubstance then if you agree that also works well.

@0xferit 0xferit changed the title Feature: Added SubstanceAmount Quantity Type [WIP] Feature: Added SubstanceAmount Quantity Type Nov 7, 2017
Ferit Tunçer added 2 commits November 7, 2017 11:36
fixed quantity filename
026b074 
changed SubstanceAmount.json to AmountOfSubstance.json and executed generate-code.bat
fixed abbreviations - added poundmole
4eff71e
@0xferit 0xferit changed the title [WIP] Feature: Added SubstanceAmount Quantity Type Feature: Added SubstanceAmount Quantity Type Nov 7, 2017
@0xferit 0xferit changed the title Feature: Added SubstanceAmount Quantity Type Feature: Added AmountOfSubstance Quantity Type Nov 7, 2017
@0xferit
Copy link
Contributor Author

0xferit commented Nov 7, 2017

I think it's ready. Can you please review?

@0xferit 0xferit changed the title Feature: Added AmountOfSubstance Quantity Type Feature: AmountOfSubstance Nov 7, 2017
@angularsen angularsen merged commit 1fad2bb into angularsen:master Nov 7, 2017
@angularsen
Copy link
Owner

Perfect! Looks good.

@0xferit 0xferit deleted the feature/add-substanceamount branch November 7, 2017 14:47
angularsen added a commit that referenced this pull request Nov 11, 2017
@angularsen
Merge origin/master
dd904f6 
Squashed commit of the following:

commit 6e23232
Author: Andreas Gullberg Larsen <[email protected]>
Date:   Sat Nov 11 12:33:12 2017 +0100

    Fix headers

    Wrong year and use (c) instead of copyright symbol since git and powershell
    frequently messes up the encoding when using it.

commit c102e3e
Author: Andreas Gullberg Larsen <[email protected]>
Date:   Sat Nov 11 12:25:18 2017 +0100

    Regenerate code from PRs

commit c1f83dd
Author: Ferit Tunçer <[email protected]>
Date:   Sat Nov 11 13:55:29 2017 +0300

    Add quantity Entropy (#312)

commit 90d5f93
Author: Ferit Tunçer <[email protected]>
Date:   Sat Nov 11 13:43:12 2017 +0300

    Fix LapseRate Units (#321)

commit c94c1d2
Author: Ferit Tunçer <[email protected]>
Date:   Fri Nov 10 20:43:44 2017 +0300

    Delete duplicate quantity SubstanceAmount (#317)

commit 471d2fc
Author: Andreas Gullberg Larsen <[email protected]>
Date:   Thu Nov 9 21:25:20 2017 +0100

    Add Equals(T other, T maxError), obsolete Equals(T other) for Double quantities

    Equality comparison is not safe with System.Double as internal representation.
    Decimal quantities (Power, Information) still allow equality.
    Add test on new Equals() method.

commit e0eb3f0
Author: Andreas Gullberg Larsen <[email protected]>
Date:   Thu Nov 9 19:57:32 2017 +0100

    UnitsNet: 3.78.0

commit 041a53e
Author: Ferit Tunçer <[email protected]>
Date:   Thu Nov 9 21:54:51 2017 +0300

    Add quantity LapseRate (#316)

commit 8a4d648
Author: Andreas Gullberg Larsen <[email protected]>
Date:   Tue Nov 7 15:34:47 2017 +0100

    Fix email address

commit aa9a99a
Author: Ferit Tunçer <[email protected]>
Date:   Tue Nov 7 23:43:11 2017 +0300

    Add micropascalseconds (#314)

commit 0a585b8
Author: Ferit Tunçer <[email protected]>
Date:   Tue Nov 7 17:45:26 2017 +0300

    Add MolarEntropy (#310)

commit e51cd52
Author: Ferit Tunçer <[email protected]>
Date:   Tue Nov 7 17:04:30 2017 +0300

    Add MolarEnergy (#309)

commit 1fad2bb
Author: Ferit Tunçer <[email protected]>
Date:   Tue Nov 7 17:02:05 2017 +0300

    Add AmountOfSubstance (#304)

commit 6800561
Author: Ferit Tunçer <[email protected]>
Date:   Tue Nov 7 08:53:13 2017 +0300

    Add MolarMass Quantity Type (#305)

    * added MolarMass
    * added russian abbreviations

    NanogramPerMole to KilogramPerMole convertion tests were failing, so we set the tolerance to 1e-3 like in massTests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xferit @angularsen