-
-
Notifications
You must be signed in to change notification settings - Fork 76
Update to the latest dev-support defaults #339
Conversation
Make it match the defaults coming from dev-support.
ff97730
to
16c23c0
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.
Thanks, @elia. I left some comments. Feel free to merge once you review and decide.
README.md
Outdated
### Updating the changelog | ||
|
||
Before and after releases the changelog should be updated to reflect the up-to-date status of | ||
the project: | ||
|
||
```shell | ||
bin/rake changelog | ||
git add CHANGELOG.md | ||
git commit -m "Update the changelog" | ||
``` | ||
|
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.
Do we need that? That's also part of the release instructions linked below.
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.
It comes from the dev-support update script, I agree it's optional, I'll remove it and update it upstream.
`git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) } | ||
end | ||
spec.files = files.grep_v(%r{^(test|spec|features)/}) | ||
spec.test_files = files.grep(%r{^(test|spec|features)/}) |
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.
What about removing test_files
? See rubocop/rubocop#10675
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.
I'll open an issue on solidus_dev_support, but I would also point out that sometimes test files are useful inline documentation, granted that if they include a 50MB image as a fixture that shouldn't probably be part of the package.
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.
I'm good with merging this, though we still need to address @waiting-for-dev 's feedback. We'll also need to make the corresponding updates to solidus_dev_support.
Also normalizes a few things to better match the defaults coming from dev-support.
31a7b69
to
1ba7bda
Compare
Summary
Just a few cleanups from running
bundle exec solidus extension .
.Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):