Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

feat: Add link to user's open-sauced goals #1016

Merged
merged 2 commits into from
May 25, 2021
Merged

feat: Add link to user's open-sauced goals #1016

merged 2 commits into from
May 25, 2021

Conversation

mtfoley
Copy link
Contributor

@mtfoley mtfoley commented May 25, 2021

What type of PR is this? (check all applicable)

  • ♻️ Refactor
  • ✨ Feature
  • 🐛 Bug Fix
  • 👷 Optimization
  • 📝 Documentation Update
  • 🔖 Release
  • 🚩 Other

Description

#948 by adding a link to user's newly created open-sauced-goals repo. As mentioned in issue commentary, redirect to this repo after installation steps may require passing state to app's OAuth flow along with configuration changes to the open-sauced GitHub app itself.

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

Added to documentation?

  • 📜 readme
  • 📜 contributing.md
  • 📓 docs
  • 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

check

src/components/CreateGoals.js Outdated Show resolved Hide resolved
Copy link
Sponsor Contributor

@filiptronicek filiptronicek 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 to me! ✅

Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

Just this one update and I think we can merge.

<small>
<em>You own all your data saved while saucin.</em>
</small>
</React.Fragment>
);
}

function InstallApp() {
function InstallApp({user}) {
const repoUrl = `https://github.com/${user.login}/open-sauced-goals`;
Copy link
Member

Choose a reason for hiding this comment

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

This should link to open-sauced/goals-template instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we don't need the user context then?

Copy link
Member

Choose a reason for hiding this comment

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

Wow, reviewing this on stream had me miss-read this. I event tested it wrong. This is good to go as is. The context of the user is needed.

Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

Sorry for the consufion. This LGTM

@bdougie bdougie merged commit 2b28132 into open-sauced:main May 25, 2021
@mtfoley mtfoley deleted the 948-link-to-open-sauced-goals branch June 4, 2021 12:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants