Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Implement skills (the migration-skill) and remove support for legacy assets in meta #278
Conversation
sergiusens
referenced this pull request
Jan 28, 2016
Closed
Remove legacy formatting and implement skills #275
zyga
reviewed
Jan 28, 2016
| + enum: | ||
| + - migration-skill | ||
| + security-policy: | ||
| + ref: "#definitions/security-policy" |
zyga
reviewed
Jan 28, 2016
| +def _copy_security_profiles(meta_dir, uses): | ||
| + # TODO: remove once skills are implemented. | ||
| + for slot in uses: | ||
| + if uses[slot]['type'] != 'migration-skill': |
zyga
Jan 28, 2016
Contributor
if slot name itself is migration-skill then this should work too (type defaults to slot name)
sergiusens
Jan 28, 2016
Collaborator
Right, I made type mandatory in the schema (I think). Will look into how this can be done
zyga
reviewed
Jan 28, 2016
| + } | ||
| + self.config_data['uses'] = { | ||
| + 'migration': { | ||
| + 'type': 'migration-skillz', |
zyga
Jan 28, 2016
Contributor
skillz?
if this is a negative test I'd rather make this more explicit.
|
A few comments inline, otherwise +1 |
|
@zyga thanks for the review |
kyrofa
reviewed
Jan 29, 2016
| + | ||
| + if 'uses' in config_data: | ||
| + snap_yaml['uses'] = \ | ||
| + _copy_security_profiles(meta_dir, config_data['uses']) |
kyrofa
Jan 29, 2016
Member
Should you also copy over the offers here since, as you mentioned, you don't want to block them from being used even if we don't have a good definition for them?
sergiusens
Jan 29, 2016
Collaborator
There are no offers to copy. The migration-skill only applies to uses. In theory it could for frameworks, but we removed support for them in snapcraft.
This code will go away as soon as skill types start landing into the os and the migration-skill is gone.
|
Yeah this looks good to me. Consider changing the PR title, but |
sergiusens
changed the title from
Feature/1539122/implement skills
to
Implement skills (the migration-skill) and remove support for legacy assets in meta
Jan 29, 2016
mvo5
reviewed
Jan 29, 2016
| + uniqueItems: true | ||
| + items: | ||
| + type: string | ||
| + offers: |
mvo5
reviewed
Jan 29, 2016
| @@ -661,6 +661,12 @@ def test_valid_security_policy_for_apps(self): | ||
| self.data['apps'] = { | ||
| 'app1': { | ||
| 'command': 'binary', | ||
| + 'uses': ['migration'], |
mvo5
Jan 29, 2016
Contributor
It says uses "migration" but the skill is called "file-migration" below?
|
Looks good, just one question in the tests. |
|
Confirmed working on the latest image |
added a commit
that referenced
this pull request
Feb 2, 2016
sergiusens
merged commit 78ca2b2
into
snapcore:master
Feb 2, 2016
sergiusens
deleted the
sergiusens:feature/1539122/implement_skills
branch
Feb 2, 2016
pushed a commit
to smoser/snapcraft
that referenced
this pull request
Sep 14, 2016
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
sergiusens commentedJan 28, 2016
This adds functionality for the migration-skill and effectively removes
support for caps, security-override, security-policy and security-template
as part of the app definition.
In addition to that, package.yaml and readme.md support is also removed.
LP: #1539122