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

Implement changes proposed in discussion of #343 #346

Merged
merged 1 commit into from
Apr 18, 2017

Conversation

PTraeder
Copy link
Contributor

Change units as proposed by @Silmathoron in the discussion of #343

Signed-off-by: Philip Traeder philip.traeder@rwth-aachen.de

Signed-off-by: Philip Traeder <philip.traeder@rwth-aachen.de>
@PTraeder PTraeder mentioned this pull request Apr 11, 2017
@DimitriPlotnikov
Copy link
Contributor

Is fine for me.

@jougs
Copy link
Contributor

jougs commented Apr 12, 2017

This also looks good to me, although I'm also confused that some variables are not used. This should probably be checked.

@DimitriPlotnikov
Copy link
Contributor

@jougs I was already thinking about a warning regarding unused variables. However, we will tackle this in the next model review iteration, after the physical unit system will be entirely settled.

@DimitriPlotnikov
Copy link
Contributor

@PTraeder Can you check for unused variables in these models?

@PTraeder
Copy link
Contributor Author

Its on my todolist now.

Copy link
Member

@Silmathoron Silmathoron left a comment

Choose a reason for hiding this comment

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

All good for me! 👍

@DimitriPlotnikov
Copy link
Contributor

@PTraeder give me sign when it is done. I'll merge the PR then.

@DimitriPlotnikov DimitriPlotnikov merged commit 0e5ad3c into nest:master Apr 18, 2017
@PTraeder PTraeder deleted the model-review branch May 10, 2017 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants