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

Modifications to the Manubot section #186

Merged
merged 19 commits into from Apr 5, 2019

Conversation

dhimmel
Copy link
Contributor

@dhimmel dhimmel commented Mar 29, 2019

Work in progress.

This PR will address the following issues:

@agitter let me know if there are any other loose ends that this PR could address/

Copy link
Collaborator

@agitter agitter left a comment

Choose a reason for hiding this comment

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

If you include #155, this could also close #104. It can also make sure we settle on a way to refer to manubot/rootstock or Rootstock and use it consistently elsewhere.

content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
@dhimmel dhimmel mentioned this pull request Apr 2, 2019
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
@dhimmel
Copy link
Contributor Author

dhimmel commented Apr 2, 2019

@agitter can you review now. Also should we add the contribution video in this PR or another?

Copy link
Collaborator

@agitter agitter left a comment

Choose a reason for hiding this comment

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

The new paragraphs and structure are a big improvement. I have several minor suggestions, mostly in the interest of keeping the descriptions accurate but not overly technical.

content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Show resolved Hide resolved
content/02.main-text.md Show resolved Hide resolved
content/02.main-text.md Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
content/02.main-text.md Outdated Show resolved Hide resolved
@agitter
Copy link
Collaborator

agitter commented Apr 5, 2019

@dhimmel I re-reviewed and resolved all of my old comments that have been addressed. There are still a few previous questions and one new comment. Can you please resolve those conversations if you do not think a change is needed or make updates if you do?

@dhimmel
Copy link
Contributor Author

dhimmel commented Apr 5, 2019

@agitter somehow I overlooked several of your comments. Everything should now be addressed except for the one-sentence-per-line paragraph, which may need further discussion.

@agitter
Copy link
Collaborator

agitter commented Apr 5, 2019

Thanks @dhimmel, your latest comments and commit addressed everything. You can proceed to merge after resolving the new conflict.

Resolve conflict & add missing newline
@dhimmel dhimmel merged commit dd08829 into greenelab:master Apr 5, 2019
agitter pushed a commit that referenced this pull request Apr 5, 2019
This build is based on
dd08829.

This commit was created by the following Travis CI build and job:
https://travis-ci.org/greenelab/meta-review/builds/516248509
https://travis-ci.org/greenelab/meta-review/jobs/516248510

[ci skip]

The full commit message that triggered this build is copied below:

Major revisions to the Manubot features section

Merges #186

Closes #76
Closes #104
Closes #128
Closes #141
Closes #155
Closes #174
agitter pushed a commit that referenced this pull request Apr 5, 2019
This build is based on
dd08829.

This commit was created by the following Travis CI build and job:
https://travis-ci.org/greenelab/meta-review/builds/516248509
https://travis-ci.org/greenelab/meta-review/jobs/516248510

[ci skip]

The full commit message that triggered this build is copied below:

Major revisions to the Manubot features section

Merges #186

Closes #76
Closes #104
Closes #128
Closes #141
Closes #155
Closes #174
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

2 participants