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

Better file conflict message #766

Merged

Conversation

sergiusens
Copy link
Collaborator

When parts conflict the message is plain and blunt, we
should do some hand holding.

LP: #1617390

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

When parts conflict the message is plain and blunt, we
should do some hand holding.

LP: #1617390

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


class SnapcraftError(Exception):
"""Base class for all storeapi exceptions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy and paste typo from storeapi/errors.py?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

indeed

@sergiusens
Copy link
Collaborator Author

thanks @evandandrea I'll apply some fixes from your comments. Is the messaging ok now or think it still needs improvements?

@evandandrea
Copy link
Contributor

The messaging is much improved, thank you.

'- `snap`\n'
'- `organize`\n\n'
'Learn more about these part keywords by running '
'`snapcraft help plugins`'
Copy link
Contributor

@kyrofa kyrofa Aug 30, 2016

Choose a reason for hiding this comment

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

This message is wonderful, but if there are more than a few files in that conflict list I could see it becoming less digestible. A little whitespace might help-- compare this:

Parts 'foo' and 'bar' have the following file paths in common which have different contents:
/home/foo/bar/baz/parts/qux/install/usr/include/my_include1.h
/home/foo/bar/baz/parts/qux/install/usr/include/my_include2.h
/home/foo/bar/baz/parts/qux/install/usr/include/my_include3.h
/home/foo/bar/baz/parts/qux/install/usr/include/my_include4.h

Snapcraft offers some capabilities to solve this the following keywords:
- `filesets`
- `stage`
- `snap`
- `organize`

Learn more about these part keywords by running `snapcraft help plugins`

With this:

Parts 'foo' and 'bar' have the following file paths in common which have different contents:
    /home/foo/bar/baz/parts/qux/install/usr/include/my_include1.h
    /home/foo/bar/baz/parts/qux/install/usr/include/my_include2.h
    /home/foo/bar/baz/parts/qux/install/usr/include/my_include3.h
    /home/foo/bar/baz/parts/qux/install/usr/include/my_include4.h

Snapcraft offers some capabilities to solve this the following keywords:
    - `filesets`
    - `stage`
    - `snap`
    - `organize`

Learn more about these part keywords by running `snapcraft help plugins`

@sergiusens
Copy link
Collaborator Author

@kyrofa point taken!

@kyrofa
Copy link
Contributor

kyrofa commented Aug 30, 2016

👍 from me! Once the tests pass.

'Learn more about these part keywords by running '
'`snapcraft help plugins`'
)
self.assertThat(exception.output, Contains(expected_help))
Copy link
Contributor

Choose a reason for hiding this comment

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

update the copyright year, please

@sergiusens sergiusens merged commit 211e8e5 into canonical:master Aug 30, 2016
@sergiusens sergiusens deleted the bugfix/1617390/conflict_message branch August 30, 2016 19:43
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
When parts conflict the message is plain and blunt, we
should do some hand holding.

LP: #1617390

Signed-off-by: Sergio Schvezov <sergio.schvezov@ubuntu.com>
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

4 participants