Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Implemented `snapcraft release` #648
Conversation
sergiusens
added some commits
Jul 11, 2016
elopio
reviewed
Jul 12, 2016
| + | ||
| +class ReleaseTestCase(integration_tests.TestCase): | ||
| + | ||
| + def _update_name_and_version(self, project_dir, name=None, version=None): |
elopio
reviewed
Jul 12, 2016
| + if text: | ||
| + text = '{}, {!r}'.format(text, channel) | ||
| + else: | ||
| + text = '{!r}'.format(channel) |
elopio
Jul 12, 2016
Member
text = ', '.join(['{!r}'.format(channel) for channel in opened_channels[:-1]])
elopio
reviewed
Jul 12, 2016
| + return 'The {} channels are now open.'.format(text) | ||
| + | ||
| + | ||
| +def _get_text_for_channel(channel): |
elopio
Jul 12, 2016
Member
This should return a tuple of three items, not a list. So instead of [channel, version, revision], use (channel, version, revision)
elopio
reviewed
Jul 12, 2016
| + tabulated_channels = tabulate(parsed_channels, | ||
| + headers=['Channel', 'Version', 'Revision']) | ||
| + # This does not look good in green so we print instead | ||
| + print('{}'.format(tabulated_channels)) |
elopio
Jul 12, 2016
Member
Why not just print(tabulated_channels) ?
Using print will probably come back and bite us when we fix the bug about saving the logs to a file. At that point, I suppose we should log with debug level an untabulated version of the channels. I'm ok leaving this for later.
sergiusens
Jul 12, 2016
Collaborator
The format is a left over from when I was trying to not use tabulate so nice catch
elopio
reviewed
Jul 12, 2016
| + | ||
| + __FMT_NOT_REGISTERED = ( | ||
| + 'Sorry, try `snapcraft register {snap_name}` before trying to ' | ||
| + 'release or choose an existing revision.') |
elopio
Jul 12, 2016
Member
I'm not convinced about this message, mainly because of the second part. I think I would just remove "or choose an existing revision". Or, it could say "or choose an already registered snap name". I'm not sure if that's better though.
sergiusens
Jul 12, 2016
Collaborator
Well the error shows up for both and I already requested OLS to provide better errors for when each scenario for a 404 happens
elopio
reviewed
Jul 12, 2016
| @@ -288,7 +292,7 @@ def _handle_upload_request(self): | ||
| int(self.headers['Content-Length'])).decode('utf8') | ||
| data = json.loads(string_data) | ||
| logger.debug( | ||
| - 'Handling registration request with content {}'.format(data)) | ||
| + 'Handling upload request with content {}'.format(data)) |
|
I'm like +0.8 here. I left a few comments for discussion. |
sergiusens
added some commits
Jul 12, 2016
|
Here's my pending +0.2. Thanks! |
sergiusens
merged commit 3567bd6
into
snapcore:master
Jul 13, 2016
sergiusens
deleted the
sergiusens:feature/1601851/snapcraft-release
branch
Jul 13, 2016
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
sergiusens commentedJul 12, 2016
No description provided.