-
Notifications
You must be signed in to change notification settings - Fork 47
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
WIP - Update formula to follow template guidelines #54
Conversation
docs/CONTRIBUTING.rst
Outdated
|
||
... | ||
|
||
BREAKING CHANGE: With the removal of all of the `.sls` files under |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its unclear to me if template
should be renamed to packages
in these contrib docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither to me. Changing it, jic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@noelmcloughlin @javierbertoli Across all semantic-release
PRs, we've just left this as template
, in order to minimise the number of changes as much as possible. It's not necessary here and it ends up leading to more false positives when diffing over time. Do you mind changing this back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. thanks @javierbertoli
Will you be splitting states into install.sls
and clean.sls
states for each package type (pkg, gem, pip, etc) in a future PR?
ba9291e
to
6fde660
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of inline comments and a few more here.
- Commit message formatting so that it gets included in the
Changelog
:Update Debian family to require python3-pip instead of python-pip
- Travis errors:
- Initial version number discrepancies (we need to decide the right way forward):
- Comparative spreadsheet suggests
v0.7.0
-- which I've used for the release at https://github.com/saltstack-formulas/packages-formula/releases/tag/v0.7.0 - Current
CHANGELOG.rst
goes up tov0.1.0
-- this file should be merged into the new changelog once it's automatically created FORMULA
file containsv0.8.1
-- but this will get overwritten, in any case
- Comparative spreadsheet suggests
docs/CONTRIBUTING.rst
Outdated
|
||
... | ||
|
||
BREAKING CHANGE: With the removal of all of the `.sls` files under |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@noelmcloughlin @javierbertoli Across all semantic-release
PRs, we've just left this as template
, in order to minimise the number of changes as much as possible. It's not necessary here and it ends up leading to more false positives when diffing over time. Do you mind changing this back?
docs/README.rst
Outdated
|
||
.. contents:: | ||
:local: | ||
:local: | ||
|
||
``packages`` | ||
------------ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the states mentioned under Available states
will need to use ^
as the heading symbol, otherwise they're not indented under the table of contents, as shown when viewing the current file:
@javierbertoli I've added four commits to this PR in my own fork, to test out This includes the fixes I mentioned above inline and also gets the Debian test run working again by updating the Let me know if you want any/all of those commits added to this PR. |
@javierbertoli #61 has been merged, so feel free to introduce your customisations from this PR on top of it. |
No description provided.