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

Rewrite sections 1 and 2 of README #209

Merged
merged 1 commit into from
Jul 2, 2020
Merged

Rewrite sections 1 and 2 of README #209

merged 1 commit into from
Jul 2, 2020

Conversation

rouille
Copy link
Collaborator

@rouille rouille commented Jun 25, 2020

Purpose

Rewrite sections 1 and 2 of README.

What is the code doing?

There is no code

Where to look

The README

Time estimate

I cannot really tell. Hopefully my english is not too bad and there is enough information. Feel free to make any corrections and more information.

@rouille rouille added the documentation Documentation related to package label Jun 25, 2020
@rouille rouille added this to the Black Lives Matter milestone Jun 25, 2020
@danielolsen danielolsen self-requested a review July 1, 2020 00:40
README.md Outdated
@@ -347,7 +391,7 @@ which sets additional curtailment for particular resource types in all regions.

Once we the regional target information and scenario-specific resource
Copy link

Choose a reason for hiding this comment

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

I know the point of this PR was to review Sections 1 and 2, but in skimming the rest of the README, I noticed that there seems to be a word or two missing here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this the only typo you noticed @lsmithiv ?

Copy link

Choose a reason for hiding this comment

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

I believe I called out all the ones I noticed in Sections 1 and 2. In skimming the other sections, nothing jumped out at me, but I could go through them a little more carefully to be sure

@danielolsen danielolsen self-requested a review July 1, 2020 20:56
Copy link
Contributor

@danielolsen danielolsen left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for doing this update! We will try to be more conscientious about updating the README when we add/modify features.

@rouille rouille force-pushed the update_readme branch 3 times, most recently from 1af7aaa to 00a099e Compare July 2, 2020 04:13
@rouille
Copy link
Collaborator Author

rouille commented Jul 2, 2020

I fixed the description of the gencost attribute of the Grid class in section 2

be found at this Dropbox location:

Dropbox(IVL)\Results\DataAnalysis\RenewablesScalingForScenarios\Clean Energy Capacity Planning Framework.pptx
Dropbox(IVL)/Results/DataAnalysis/RenewablesScalingForScenarios/Clean Energy Capacity Planning Framework.pptx
Copy link

@lsmithiv lsmithiv Jul 2, 2020

Choose a reason for hiding this comment

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

Once the repository is public, will we still want to refer people to the Dropbox? I know this isn't a problem now, since everything is still internal, I was just curious.

Copy link
Collaborator Author

@rouille rouille Jul 2, 2020

Choose a reason for hiding this comment

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

No. There should not be any link to Dropbox, private or public. If a file is needed to run a code it should be enclosed in a data folder in the code base and version controlled.

Copy link

Choose a reason for hiding this comment

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

Thank you for clarifying

Copy link
Contributor

Choose a reason for hiding this comment

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

Damn, I missed the feta folder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My phone tricked me!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not so sure. In my mind, 'feta' is usually a good answer to most questions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nathan has a t-shirt that says pizzas is always the answer. I will get you the feta one then.

@danielolsen
Copy link
Contributor

@lsmithiv it is on our to-do list to simplify the interface described in Section 3 (see #195), we will re-write the README when we refactor (after the July review).

@lsmithiv
Copy link

lsmithiv commented Jul 2, 2020

@lsmithiv it is on our to-do list to simplify the interface described in Section 3 (see #195), we will re-write the README when we refactor (after the July review).

Sounds good, thank you for letting me know!

@rouille rouille merged commit e5a2681 into develop Jul 2, 2020
@rouille rouille deleted the update_readme branch July 2, 2020 23:34
@ahurli ahurli mentioned this pull request Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation related to package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants