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

Add template for new playpen projects #83

Merged
merged 7 commits into from Jun 20, 2016

Conversation

Projects
None yet
3 participants
@didrocks
Member

didrocks commented Jun 20, 2016

  • Use 2 spaces for .yaml files.
  • "apps" (user visible parts) are always second, the build details recipes (parts) are afterwards.
  • Explain some 80 characters rule for line breakage, including for list.
  • Point CONTRIBUTING to the new template, containing the template README as well.
  • Change ci-run to skip the template directory.
@@ -16,4 +16,4 @@ on the projects add to it. For your project to be added, it is important that:
Please note that we are going to consider removing a non-working snap from the
repo if we can't get it to work after a month without updates.

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

s/repo/playpen/

Just something picky. ;)

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

s/repo/playpen/

Just something picky. ;)

This comment has been minimized.

@didrocks

didrocks Jun 20, 2016

Member

This is in the contextual diff, I'll do a direct typo fix following this PR, nice addition! :)

@didrocks

didrocks Jun 20, 2016

Member

This is in the contextual diff, I'll do a direct typo fix following this PR, nice addition! :)

Show outdated Hide outdated snap-template/snapcraft.yaml
# Lines for yaml files (including description) should be under
# 80 characters. If more if needed, they should be
# broken up in multiple stenzas.
# That way, list like [foo, bar, baz] should be broken up into:

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

For example, a list like [foo, bar, baz] should be broken up into:

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

For example, a list like [foo, bar, baz] should be broken up into:

This comment has been minimized.

@didrocks

didrocks Jun 20, 2016

Member

Better, indeed, changed and pushed. Thanks!

@didrocks

didrocks Jun 20, 2016

Member

Better, indeed, changed and pushed. Thanks!

Show outdated Hide outdated snap-template/snapcraft.yaml
@@ -0,0 +1,30 @@
name: my-snap # the name of the snap
version: 0 # the version of the snap

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

I would suggest changing to 0.1 to point out that they can have decimals in the version.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

I would suggest changing to 0.1 to point out that they can have decimals in the version.

Show outdated Hide outdated snap-template/snapcraft.yaml
name: my-snap # the name of the snap
version: 0 # the version of the snap
summary: This is my-snap's summary # 79 char long summary
description: |

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

Why is it only a |? I've always put something on this line, but I could just not know what I'm talking about ;)

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

Why is it only a |? I've always put something on this line, but I could just not know what I'm talking about ;)

This comment has been minimized.

@didrocks

didrocks Jun 20, 2016

Member

otherwise, it's not going to be a valid yaml file (as per spec), I find is as well cleaner to have then all the text aligned

@didrocks

didrocks Jun 20, 2016

Member

otherwise, it's not going to be a valid yaml file (as per spec), I find is as well cleaner to have then all the text aligned

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

ah I see, I learned something new :)

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

ah I see, I learned something new :)

This comment has been minimized.

Show outdated Hide outdated snap-template/snapcraft.yaml
parts:
my-part: # Replace with a part name of your liking
# Get more information about plugins by running

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

If it's going to be on different lines from the command, I would suggest adding a colon at the end of this line

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

If it's going to be on different lines from the command, I would suggest adding a colon at the end of this line

This comment has been minimized.

@didrocks

didrocks Jun 20, 2016

Member

correct! Good catch

@didrocks

didrocks Jun 20, 2016

Member

correct! Good catch

Show outdated Hide outdated snap-template/snapcraft.yaml
# Get more information about plugins by running
# snapcraft help plugins
# and more information about the available plugins
# by running

This comment has been minimized.

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

ditto, and this could be on the line above

@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

ditto, and this could be on the line above

This comment has been minimized.

@didrocks

didrocks Jun 20, 2016

Member

done as well. This come from standard snapcraft init, we should change it btw :)

@didrocks

didrocks Jun 20, 2016

Member

done as well. This come from standard snapcraft init, we should change it btw :)

didrocks added some commits Jun 20, 2016

Fix more from template
Note that this should be fixed as well in "snapcraft init"
Fix version and enforce it being a string
That way, we avoid https://launchpad.net/bugs/1586988 where 2.10 is modified
into 2.1.
@dholbach

This comment has been minimized.

Show comment
Hide comment
@dholbach

dholbach Jun 20, 2016

Collaborator

Nice work! 👍

Collaborator

dholbach commented Jun 20, 2016

Nice work! 👍

@tsimonq2

This comment has been minimized.

Show comment
Hide comment
@tsimonq2

tsimonq2 Jun 20, 2016

Contributor

👍

Contributor

tsimonq2 commented Jun 20, 2016

👍

@didrocks

This comment has been minimized.

Show comment
Hide comment
@didrocks

didrocks Jun 20, 2016

Member

Ok, let's go for a merge then!

Member

didrocks commented Jun 20, 2016

Ok, let's go for a merge then!

@dholbach dholbach merged commit 71a5164 into ubuntu:master Jun 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment