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] Added link for tedana NeuroStars tag. #608

Merged
merged 6 commits into from
Sep 28, 2020

Conversation

aykhojandi
Copy link
Contributor

@aykhojandi aykhojandi commented Sep 22, 2020

Closes #593 .

Changes proposed in this pull request:

  • Update support.rst file to include tedana tag link.
  • Question: Should we also edit Line 7 to say "If you would like ... tedana tag." instead of "... multi-echo tag." ?(@jbteves and I were wondering this, given that we now want to point tedana-specific questions to the tedana tag).

@jbteves
Copy link
Collaborator

jbteves commented Sep 22, 2020

Hey @aykhojandi, thanks for your contribution! Two quick things:

  1. Do you mind putting [DOC] at the start, rather than end, of your PR
  2. Do you mind making this a Draft PR? Since we're unsure if it's actually ready for review.

@aykhojandi aykhojandi marked this pull request as draft September 22, 2020 21:20
@aykhojandi aykhojandi changed the title Added link for tedana NeuroStars tag. [DOC] [DOC] Added link for tedana NeuroStars tag. Sep 22, 2020
@aykhojandi
Copy link
Contributor Author

aykhojandi commented Sep 22, 2020 via email

@tsalo
Copy link
Member

tsalo commented Sep 22, 2020

Question: Should we also edit Line 7 to say "If you would like ... tedana tag." instead of "... multi-echo tag." ?(@jbteves and I were wondering this, given that we now want to point tedana-specific questions to the tedana tag).

I would say something along the lines of "Please tag any posts about tedana usage with the tedana tag and any general questions about multi-echo fMRI with the multi-echo tag." Not that exactly, but basically distinguishing the two tags.

@codecov
Copy link

codecov bot commented Sep 22, 2020

Codecov Report

Merging #608 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #608   +/-   ##
=======================================
  Coverage   93.53%   93.53%           
=======================================
  Files          26       26           
  Lines        1965     1965           
=======================================
  Hits         1838     1838           
  Misses        127      127           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0b81bc...ee28a17. Read the comment docs.

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

A couple of minor adjustments to link to NeuroStars with each of the tags.

docs/support.rst Outdated Show resolved Hide resolved
docs/support.rst Outdated Show resolved Hide resolved
docs/support.rst Outdated Show resolved Hide resolved
aykhojandi and others added 2 commits September 22, 2020 17:48
Add 'tag' before tedana tag link

Co-authored-by: Taylor Salo <tsalo006@fiu.edu>
Added proper formatting to tedana tag in support.rst

Co-authored-by: Taylor Salo <tsalo006@fiu.edu>
@tsalo
Copy link
Member

tsalo commented Sep 24, 2020

@aykhojandi there's just one more change I'd like to see, but this PR is looking great! Do you want to mark it ready for review?

Better language clarifying tags

Co-authored-by: Taylor Salo <tsalo006@fiu.edu>
@aykhojandi aykhojandi marked this pull request as ready for review September 24, 2020 17:15
@aykhojandi
Copy link
Contributor Author

Question: Should we also edit Line 7 to say "If you would like ... tedana tag." instead of "... multi-echo tag." ?(@jbteves and I were wondering this, given that we now want to point tedana-specific questions to the tedana tag).

I would say something along the lines of "Please tag any posts about tedana usage with the tedana tag and any general questions about multi-echo fMRI with the multi-echo tag." Not that exactly, but basically distinguishing the two tags.

@aykhojandi there's just one more change I'd like to see, but this PR is looking great! Do you want to mark it ready for review?

Got it. I believe the last suggestion is now committed. I marked this ready for review. Let me know how else I can be of assistance.

@tsalo
Copy link
Member

tsalo commented Sep 24, 2020

One related thing that would be really helpful is if you could also update the issue template (.github/ISSUE_TEMPLATE/issue_template.md), which asks users to use the "multi-echo" NeuroStars tag for usage-related questions. If you could change that to "tedana" that would be awesome.

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for this!

@tsalo tsalo requested a review from jbteves September 25, 2020 18:19
Copy link
Member

@emdupre emdupre left a comment

Choose a reason for hiding this comment

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

This LGTM 👍 Thank you, @aykhojandi !

@jbteves
Copy link
Collaborator

jbteves commented Sep 28, 2020

Oops, sorry I slipped on this. People without write access need to have their PRs merged for them, right? It's been a while.

@emdupre
Copy link
Member

emdupre commented Sep 28, 2020

People without write access need to have their PRs merged for them, right?

Yes 😸 If you also approve, I'll let you pull the trigger !

Copy link
Collaborator

@jbteves jbteves left a comment

Choose a reason for hiding this comment

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

Thanks so much for this, Arman! LGTM

@jbteves jbteves merged commit e4e3e26 into ME-ICA:master Sep 28, 2020
@aykhojandi aykhojandi deleted the fix_ns_tag branch September 28, 2020 17:40
@tsalo
Copy link
Member

tsalo commented Oct 3, 2020

@all-contributors please add @aykhojandi for documentation

@allcontributors
Copy link
Contributor

@tsalo

I've put up a pull request to add @aykhojandi! 🎉

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.

Replace 'multi-echo' NeuroStars tag with 'tedana' on Support page
4 participants