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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add snapcraft documentation #11794

Merged
merged 9 commits into from Feb 2, 2018

Conversation

Projects
None yet
9 participants
@felixrieseberg
Member

felixrieseberg commented Feb 1, 2018

馃憢 I'm coming live to you from Canonical's Snapcraft summit, where some of us (Visual Studio Code, Slack, Skype, electron-forge, electron-builder, etc) have been playing with snapcraft technology.

You shouldn't need a summit to play with it, so hence some solid documentation - in line with our Windows Store and Mac App Store documentation.

@felixrieseberg felixrieseberg requested a review from electron/docs as a code owner Feb 1, 2018

@develar

This comment has been minimized.

Show comment
Hide comment
@develar

develar Feb 1, 2018

Contributor

I think note, that electron-builder/electron-forge is recommended unless there are some reasons to no use it, should be added. Because right now it not clear :) As we have discussed, for more users forge/builder way is more easy (just set target to snap and install snapcraft/docker/nothing).

Contributor

develar commented Feb 1, 2018

I think note, that electron-builder/electron-forge is recommended unless there are some reasons to no use it, should be added. Because right now it not clear :) As we have discussed, for more users forge/builder way is more easy (just set target to snap and install snapcraft/docker/nothing).

Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
@evandandrea

This is great. I've inlined some suggestions.

Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
@malept

This comment has been minimized.

Show comment
Hide comment
@malept

malept Feb 1, 2018

Member

I assume this also needs a link from docs/README.md?

Member

malept commented Feb 1, 2018

I assume this also needs a link from docs/README.md?

@felixrieseberg

This comment has been minimized.

Show comment
Hide comment
@felixrieseberg

felixrieseberg Feb 1, 2018

Member

Thanks a ton for all the feedback <3 I implemented it all.

Member

felixrieseberg commented Feb 1, 2018

Thanks a ton for all the feedback <3 I implemented it all.

felixrieseberg added some commits Feb 1, 2018

@ckerr

This comment has been minimized.

Show comment
Hide comment
@ckerr

ckerr Feb 1, 2018

Member

Evan, Leo, the price of me merging this is you buying Felix some drinks on sabdfl's dime :D

Member

ckerr commented Feb 1, 2018

Evan, Leo, the price of me merging this is you buying Felix some drinks on sabdfl's dime :D

@elopio

This comment has been minimized.

Show comment
Hide comment
@elopio

elopio Feb 1, 2018

@ckerr, consider it done my friend! 馃榿

elopio commented Feb 1, 2018

@ckerr, consider it done my friend! 馃榿

Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
@felixrieseberg

This comment has been minimized.

Show comment
Hide comment
@felixrieseberg

felixrieseberg Feb 1, 2018

Member

@ckerr Assuming that tests pass, this should now be good to go. I'll collect my beer later tonight 馃槃

<3 Thanks everyone for the editorial help!

Member

felixrieseberg commented Feb 1, 2018

@ckerr Assuming that tests pass, this should now be good to go. I'll collect my beer later tonight 馃槃

<3 Thanks everyone for the editorial help!

@kyrofa

This comment has been minimized.

Show comment
Hide comment
@kyrofa

kyrofa Feb 1, 2018

@ckerr deal! Great to see you around 馃榿 .

kyrofa commented Feb 1, 2018

@ckerr deal! Great to see you around 馃榿 .

@malept

malept approved these changes Feb 1, 2018

@zeke

zeke approved these changes Feb 1, 2018

Awesome! A few minor nits but overall this looks great.

I would suggest changing the filename to snapcraft.md. Everything in /docs/tutorial is a guide. (I know there are other files with the guide suffix.. oh well).

Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
Show outdated Hide outdated docs/tutorial/snapcraft-guide.md
@felixrieseberg

This comment has been minimized.

Show comment
Hide comment
@felixrieseberg

felixrieseberg Feb 2, 2018

Member

@zeke: Thanks! Last round of comments all implemented.

Member

felixrieseberg commented Feb 2, 2018

@zeke: Thanks! Last round of comments all implemented.

@zeke

zeke approved these changes Feb 2, 2018

@zeke zeke merged commit a033a9c into master Feb 2, 2018

8 checks passed

ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@zeke zeke deleted the snapcraft-documentation branch Feb 2, 2018

@elopio

This comment has been minimized.

Show comment
Hide comment
@elopio

elopio Feb 2, 2018

20180202_015855
@ckerr proof of compliance

elopio commented Feb 2, 2018

20180202_015855
@ckerr proof of compliance

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