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.

Sign up for GitHub

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

Jump to bottom

Comments on manuscript #54

Closed
bdsullivan opened this issue Aug 29, 2019 · 2 comments
Closed

Comments on manuscript #54

bdsullivan opened this issue Aug 29, 2019 · 2 comments

Comments

@bdsullivan
Copy link
Collaborator

1765_001.pdf

Old-school markup attached, that should probably be broken into multiple issues. Mixture of fine-grained (e.g. typos and concerns about figure legends/clarity) and broader-scale concerns (e.g. lack of definitions of technical terms, organization, and seemingly overly-explanatory language "justifying" the approach/results).

Overall, I have some qualms about the tone of the paper (e.g. what is being explained, where, and at what level of detail), and would be happy to have a call to discuss this more broadly. I also feel like the paper suffers from a lack of technical specificity in much of the narrative. I'm still not 100% sure I know what you algorithm you're using when you say "edge reconstruction", which worries me.

Let me know how you'd like to approach things from here. Thanks!
@zietzm zietzm mentioned this issue Sep 2, 2019
Merged
@cgreene cgreene mentioned this issue Sep 9, 2019
Merged
dhimmel pushed a commit that referenced this issue Sep 9, 2019
@zietzm @dhimmel
Add bdsullivan authorship (#56)
76374a3 
Merges #56
Refs #54
zietzm added a commit that referenced this issue Sep 9, 2019
@zietzm
Add bdsullivan authorship (#56) [ci skip]
0f9b936 
This build is based on
76374a3.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/greenelab/xswap-manuscript/builds/126464775
https://travis-ci.com/greenelab/xswap-manuscript/jobs/232371267

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

Add bdsullivan authorship (#56)

Merges #56
Refs #54
zietzm added a commit that referenced this issue Sep 9, 2019
@zietzm
Add bdsullivan authorship (#56) [ci skip]
735fb3c 
This build is based on
76374a3.

This commit was created by the following Travis CI build and job:
https://travis-ci.com/greenelab/xswap-manuscript/builds/126464775
https://travis-ci.com/greenelab/xswap-manuscript/jobs/232371267

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

Add bdsullivan authorship (#56)

Merges #56
Refs #54
@bdsullivan
Copy link
Collaborator Author

1851_001.pdf

Oops. Double-sided scanning fail (haven't beaten the new department's copy machine into submission yet) -- sorry for confusion. Full review attached.

@zietzm zietzm mentioned this issue Sep 9, 2019
Merged
dhimmel pushed a commit that referenced this issue Nov 12, 2022
@zietzm
revisions to address Sullivan review (#57)
9d233c8 
merges #57
refs #54
@dhimmel
Copy link
Collaborator

dhimmel commented Nov 12, 2022 •
edited

Closing now that we've merged #57. Thanks @bdsullivan for the feedback.

@dhimmel dhimmel closed this as completed Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dhimmel @bdsullivan