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

DOC: update doctr instruction #79

Merged
merged 5 commits into from
Jan 13, 2020

Conversation

ke-zhang-rd
Copy link

Reason: https://github.com/drdoctr/doctr/blob/0f19ff78c8239efcc98d417f36b0a31d9be01ba5/doctr/travis.py#L593

It's very hard to find in Travis log. Think need to mention in instruction.

@asmeurer
Copy link

FWIW, forked repos will be supported once I finish drdoctr/doctr#343, assuming that you configure doctr for that repo. The main issue is if you configure a base repo then a fork cannot work, as Travis doesn't give forks or forked pull requests access to the secure environment variables for obvious reasons.

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

I think this warning can be better represented via a warning block.

I also think the link from @asmeurer's #79 (comment) can be mentioned as well.

Ke Zhang and others added 2 commits January 10, 2020 13:27
@ke-zhang-rd ke-zhang-rd requested a review from mrakitin January 10, 2020 18:43
Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Looks good. Here are a couple of my suggestions.

Ke Zhang and others added 2 commits January 10, 2020 14:01
@ke-zhang-rd ke-zhang-rd requested a review from mrakitin January 10, 2020 19:02
Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

A note to ourselves: it's not necessary to have a blank line between .. warning:: and the text, it renders fine by sphinx:
image

@mrakitin mrakitin merged commit 5521f24 into NSLS-II:master Jan 13, 2020
@danielballan
Copy link
Contributor

Good to know. At least some directives will be confused by the lack of a blank line, as they will interpret the line beneath .. directive:: as arguments/parameters so as a matter of style/linting the blank line is probably a good habit just to avoid tripping over that when it does come up.

@mrakitin
Copy link
Member

Sounds good, will do in the future.

hotchilipowder pushed a commit to hotchilipowder/scientific-python-cookiecutter that referenced this pull request Jan 18, 2024
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.

4 participants