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

simplify getting started #1129

Closed
wants to merge 3 commits into from
Closed

Conversation

afeld
Copy link

@afeld afeld commented Nov 6, 2016

Associated Issue: #1127

Summary of Changes

There are a few commits, which are somewhat unrelated, but build on one another:

  1. 6f0f7c8: Update yarn.lock file. This is the result of running yarn install on master using yarn 0.16.1, without any other changes. I guess the last person to update dependencies hadn't committed the updates to the lock file, or something changed in yarn...?
  2. 3a3217f: Remove duplicate getting started instructions
  3. e5db7b7: Use a single command to start Firefox and the debugger process, rather than needing two terminals. This has the additional advantage of interleaving the logs from both.

Testing

  • passes npm test
  • passes npm run lint

Screenshots/Videos (OPTIONAL)

screen shot 2016-11-06 at 2 56 17 pm

@jasonLaster
Copy link
Contributor

Thanks @afeld! These are great contributions that will help with the getting started process.

This is probably a little bit more overhead, but in the future it would be helpful to make three separate PRs as each one requires a slightly different review cycle and I'd hate for one of the three to block the other two.

  1. yarn.lock - looks great (i rm'd my node_modules) and everything worked)
  2. canonical getting started guides. I like this idea, it's probably better to link to one really good doc. @clarkbw what do you think?
  3. single command
    • I like the command, but I'm not sure if i'm ready to tell people to use it yet. I'd like to try it out myself for a bit. Also, I want npm run firefox to be more bullet proof first.
    • I like the style cleanups
    • Could you remove the reference to it in the doc.

@jasonLaster jasonLaster mentioned this pull request Nov 7, 2016
@Garbee
Copy link
Contributor

Garbee commented Nov 15, 2016

I think the primary necessity from this PR currently is the documentation changes. We do need to look over everything with fresh-eyes in the point-of-view of someone who knows nothing about the project to see where we can optimize the on-boarding path. At this time though, I'd warn against introducing new tools (such as Foreman in this PR) until we have some tangible feedback from multiple people that they are having a hard time getting things going after the docs are cleaned up.

Foreman in this instance is not only adding a new dependency but also bringing along yet-another top-level file. At some point it gets confusing to even go through them much less know what most of them are for.

I'd rather we start on the documentation end here and make that clearer. Then we can listen to feedback on the process and adjust the commands as we go. If we end up hearing a lot that once things are fixed up more getting everything running is still an issue, then we can give running two npm scripts in parallel the due diligence on having that done internally for a quick-start command.

@jasonLaster
Copy link
Contributor

jasonLaster commented Nov 15, 2016

Thanks @Garbee, I think that's a persuasive case.

Lets take Aiden's recommendation and consolidate the getting started guide. Many people want one guide on the steps to get started.

Once that lands, we can re-evaluate foreman. We've landed lots of fixes to npm run firefox in the past week and i'm now confident it will work on all computers and in any order (starting firefox first or second)... Lets let this bake and then re-evaluate foreman.

Thanks @afeld for kicking off this discussion. The OSS workshop was a great place to test this setup in the wild!

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.

3 participants