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

More links to .com #1589

Merged
merged 5 commits into from Jul 19, 2018

Conversation

3 participants
@simurai
Member

simurai commented Jul 18, 2018

Description of the Change

This adds a bunch of links to github.com on the PR detail page.

  • PR title (no hover effect)
  • A link in the footer
  • Commit sha
  • Timestamp of comments

screen shot 2018-07-19 at 5 27 48 pm

Alternate Designs

An alternative would be to open everything inside Atom, but currently that's not possible/feassable.

Benefits

Users have a way to continue on .com to get more infos or do certain actions.

Possible Drawbacks

None

Applicable Issues

Part of #1582

@simurai simurai changed the title from More links to .com to [WIP] More links to .com Jul 18, 2018

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jul 18, 2018

Coverage Status

Coverage increased (+0.08%) to 79.577% when pulling 950e4e1 on sm/links into ac4d3a1 on master.

coveralls commented Jul 18, 2018

Coverage Status

Coverage increased (+0.08%) to 79.577% when pulling 950e4e1 on sm/links into ac4d3a1 on master.

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Jul 18, 2018

Member

I'm stuck adding more links. For example, I tried to turn the sha into a link to the commit with href={commit.commitUrl}. I also added commitUrl in graphql query, but it doesn't output anything.

I saw a bunch of lib/views/__generated__/....Query.graphql.js. Do these need to be generated somehow? Anyways, I'll add the help-needed label, if someone wants to continue. 🙇

Member

simurai commented Jul 18, 2018

I'm stuck adding more links. For example, I tried to turn the sha into a link to the commit with href={commit.commitUrl}. I also added commitUrl in graphql query, but it doesn't output anything.

I saw a bunch of lib/views/__generated__/....Query.graphql.js. Do these need to be generated somehow? Anyways, I'll add the help-needed label, if someone wants to continue. 🙇

@smashwilson

This comment has been minimized.

Show comment
Hide comment
@smashwilson

smashwilson Jul 18, 2018

Member

Do these need to be generated somehow?

Ahhhh, yup. If you edit graphql literals, you need to run npm run relay to recompile all of the GraphQL fragments. It's... weird.

Member

smashwilson commented Jul 18, 2018

Do these need to be generated somehow?

Ahhhh, yup. If you edit graphql literals, you need to run npm run relay to recompile all of the GraphQL fragments. It's... weird.

@smashwilson

This comment has been minimized.

Show comment
Hide comment
@smashwilson

smashwilson Jul 18, 2018

Member

(We've just been checking the generated GraphQL files in to version control, as well.)

Member

smashwilson commented Jul 18, 2018

(We've just been checking the generated GraphQL files in to version control, as well.)

@simurai simurai changed the title from [WIP] More links to .com to More links to .com Jul 19, 2018

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Jul 19, 2018

Member

you need to run npm run relay to recompile all of the GraphQL fragments.

That worked! 🙌 Ok, I think this is ready.

We could also link avatars and usernames and more stuff, but that's a bit more complex when there are multiple names involved. I also ran into failing specs, hence the force-pushes ☝️.

Member

simurai commented Jul 19, 2018

you need to run npm run relay to recompile all of the GraphQL fragments.

That worked! 🙌 Ok, I think this is ready.

We could also link avatars and usernames and more stuff, but that's a bit more complex when there are multiple names involved. I also ran into failing specs, hence the force-pushes ☝️.

@simurai simurai requested a review from smashwilson Jul 19, 2018

@simurai simurai added this to In Progress 🔧 in Stability Sprint : 23 July - 3 August 2018 : v0.19.0 via automation Jul 19, 2018

@simurai simurai moved this from In Progress 🔧 to QA Review 🔬 in Stability Sprint : 23 July - 3 August 2018 : v0.19.0 Jul 19, 2018

@smashwilson

👍 🎨

@smashwilson smashwilson added this to In Progress 🔧 in Stability Sprint : 23 July - 3 August 2018 : v0.19.0 via automation Jul 19, 2018

@smashwilson smashwilson merged commit d3ca6a8 into master Jul 19, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 79.524%
Details

Stability Sprint : 23 July - 3 August 2018 : v0.19.0 automation moved this from In Progress 🔧 to Merged ☑️ Jul 19, 2018

@smashwilson smashwilson deleted the sm/links branch Jul 19, 2018

This was referenced Aug 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment