Skip to content

Docs: Remove env_developer.yml from install guide#727

Merged
peanutfun merged 2 commits intodevelopfrom
feature/doc_install_dev_extra
May 23, 2023
Merged

Docs: Remove env_developer.yml from install guide#727
peanutfun merged 2 commits intodevelopfrom
feature/doc_install_dev_extra

Conversation

@tovogt
Copy link
Copy Markdown
Collaborator

@tovogt tovogt commented May 23, 2023 •
edited
Loading

Changes proposed in this PR:

PR Author Checklist

PR Reviewer Checklist

peanutfun
peanutfun requested changes May 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@peanutfun peanutfun left a comment

Choose a reason for hiding this comment

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

@tovogt Thanks a lot for this fix, I missed the part where the old env_developer.yml was still used. However, the information on dev and extras in general was moved into its own section, see https://climada-python.readthedocs.io/en/latest/guide/install.html#install-developer-dependencies-optional. Did you miss that part, by chance? I don't think we should display this information twice, and I also think that installing the dev requirements should not be the default instruction.

@tovogt
Copy link
Copy Markdown
Collaborator Author

tovogt commented May 23, 2023

Ah, okay, I missed that part. I thought I was in the section about developer instructions already. And the dev requirements used to be the default here before. But I'm fine with removing the extra part.

@tovogt tovogt changed the title DOC: Mention [dev] extra in install guide Docs: Remove env_developer.yml from install guide May 23, 2023
@tovogt
Copy link
Copy Markdown
Collaborator Author

tovogt commented May 23, 2023 •
edited
Loading

I added your suggested changes.

Are you aware that the Install Developer Dependencies (Optional) section claims that [test] is required to run the tests, but you already run tests as an example at the end of the previous Advanced Instructions where this extra is not installed yet?

@peanutfun
Copy link
Copy Markdown
Member

peanutfun commented May 23, 2023

@tovogt I admit, this is a bit imprecise. Executing python -m unittest climada.engine.test.test_impact does not require the test extras. These extras mostly contain "test tools" like coverage etc.

@peanutfun
Copy link
Copy Markdown
Member

peanutfun commented May 23, 2023

I tried to mention this somewhat explicitly:

Building the documentation and running the entire test suite of CLIMADA requires additional dependencies

(emphasis by me)

@tovogt Do you think this is clear enough or should we change something about that?

@tovogt
Copy link
Copy Markdown
Collaborator Author

tovogt commented May 23, 2023

Don't worry, I think it's clear enough. Thanks for considering my concerns :)

@peanutfun
Copy link
Copy Markdown
Member

peanutfun commented May 23, 2023

Great, thanks a lot for taking your time and fixing the docs! 🎉

@peanutfun peanutfun merged commit 54dd0fc into develop May 23, 2023
@emanuel-schmid emanuel-schmid deleted the feature/doc_install_dev_extra branch May 23, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peanutfun peanutfun peanutfun approved these changes

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

@tovogt @peanutfun