Skip to content
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

New command cleanbuild (using lxd) #328

Merged

Conversation

sergiusens
Copy link
Collaborator

This new command's intention is to make building with a clean
slate and easy chore.

LP: #1480144

Signed-off-by: Sergio Schvezov sergio.schvezov@ubuntu.com

@sergiusens
Copy link
Collaborator Author

Hello there, this is cleanbuild, it used to use pylxd but not anymore as the API changed, does not work with python3 with that change and does not have tests anymore.

I skipped adding an integration test as it seemed way to invasive.

"""
snapcraft cleanbuild

Uses a container using lxd to create the resulting snap.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of "Uses a container using lxd", just "Uses a lxd container".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also instead of "to create the resulting snap", just "to build the snap".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@come-maiz
Copy link
Contributor

I like this very much. I left many comments just because I'm cranky and want to start my vacations :)
I would like that invasive integration test. Maybe we can just skip it if lxd is not installed.

@sergiusens sergiusens force-pushed the feature/1480144/cleanbuild-lxd branch 2 times, most recently from f214fd3 to 6b1bb6f Compare February 19, 2016 17:46
fn = tarinfo.name
if fn.startswith('./parts/') and not fn.startswith('./parts/plugins'):
return None
elif any(fn == d for d in ('./stage', './snap', tar_filename)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find fn in ('./stage', './snap', tar_filename) more readable than this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@chipaca
Copy link
Contributor

chipaca commented Feb 19, 2016

👍

@sergiusens sergiusens force-pushed the feature/1480144/cleanbuild-lxd branch 3 times, most recently from 5e527fb to a5f5038 Compare February 20, 2016 03:05
This new command's intention is to make building with a clean
slate and easy chore.

LP: #1480144

Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com>
sergiusens added a commit that referenced this pull request Feb 23, 2016
@sergiusens sergiusens merged commit 2f61162 into canonical:master Feb 23, 2016
@sergiusens sergiusens deleted the feature/1480144/cleanbuild-lxd branch February 23, 2016 12:40
smoser pushed a commit to smoser/snapcraft that referenced this pull request Sep 14, 2016
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
…nbuild-lxd

New command cleanbuild (using lxd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants