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.

Sign up for GitHub

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

Jump to bottom

Refactor to align with template-formula; fix failures #29

Merged
merged 6 commits into from
May 6, 2020
Merged

Refactor to align with template-formula; fix failures #29

merged 6 commits into from
May 6, 2020

Conversation

noelmcloughlin
Copy link
Member

@noelmcloughlin noelmcloughlin commented Apr 16, 2020 •
edited
Loading

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

Yes,

but the existing formula is not working anyway

Related issues and/or pull requests

The formula was failing because the hardcoded 2018 release is no longer available - see
#28. This PR fixes that issue and is a rewrite to align to template-formula. Travis is passing for various Linux.

Describe the changes you're proposing

Refactored formula

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@noelmcloughlin
feat(formula): major rewrite, align to template-formula
bbbecab 
BREAKING CHANGE: Major refactor of formula to bring it in alignment with the
.  As with all substantial changes, please ensure your
existing configurations work in the ways you expect from this formula.
@noelmcloughlin
Merge branch 'master' of https://github.com/saltstack-formulas/eclips…
f19fda2 
…e-formula into refactor
@noelmcloughlin
fix(macos): add user to osfamilymap
a05dbe6
myii added a commit to myii/ssf-formula that referenced this pull request May 6, 2020
@myii
feat(formula): add eclipse-formula
4acc24d 
* saltstack-formulas/eclipse-formula#29
* https://www.eclipse.org/downloads/packages/compare
@myii myii mentioned this pull request May 6, 2020
Merged
myii added 2 commits May 6, 2020 16:29
@myii
feat(semantic-release): standardise for this formula
8922bde 
* Automated using myii/ssf-formula#177
@myii
fix(yamllint): fix all errors
cc91e18 
```bash
$ yamllint -s .
./eclipse/defaults.yaml
  55:89     error    line too long (120 > 88 characters)  (line-length)

./test/salt/javascript/pillar.sls
  34:89     error    line too long (144 > 88 characters)  (line-length)

./test/salt/java/pillar.sls
  5:89      error    line too long (92 > 88 characters)  (line-length)
  33:89     error    line too long (134 > 88 characters)  (line-length)

./test/salt/cpp/pillar.sls
  34:89     error    line too long (144 > 88 characters)  (line-length)
```
@pull-assistant
Copy link

pull-assistant bot commented May 6, 2020

Score: 0.95

Best reviewed: commit by commit

Optimal code review plan (1 warning)

     feat(formula): major rewrite, align to template-formula

     Merge branch 'master' of https://github.com/saltstack-formulas/eclipse...

fix(macos): add user to osfamilymap

...est/integration/cpp/inspec.yml 50% changes removed in feat(semantic-releas...

...st/integration/java/inspec.yml 50% changes removed in feat(semantic-releas...

...egration/javascript/inspec.yml 50% changes removed in feat(semantic-releas...

     chore(yaml): remove unused parameter

     feat(semantic-release): standardise for this formula

     fix(yamllint): fix all errors

Powered by Pull Assistant. Last update bbbecab ... cc91e18. Read the comment docs.

@myii
Copy link
Member

myii commented May 6, 2020 •
edited
Loading

@noelmcloughlin Finally, review this one and pushed two commits on top to standardise the structure. Please have a look at both and go ahead and merge if you're OK with the proposed changes.

noelmcloughlin
noelmcloughlin commented May 6, 2020
View reviewed changes
Copy link
Member Author

@noelmcloughlin noelmcloughlin left a comment

Choose a reason for hiding this comment

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

Thanks @myii for reviewing

@noelmcloughlin noelmcloughlin merged commit 69a926e into saltstack-formulas:master May 6, 2020
@myii
Copy link
Member

myii commented May 6, 2020

@noelmcloughlin There's another yamllint error introduced in your last commit and are you sure you mean to use None rather than null or even ~?

@myii
Copy link
Member

myii commented May 6, 2020

https://travis-ci.com/github/saltstack-formulas/eclipse-formula/jobs/328737888#L303-L305

$ yamllint -s .
./pillar.example
  25:18     warning  too few spaces before comment  (comments)

@myii
Copy link
Member

myii commented May 6, 2020

Forcing the release, so that we make ourselves official here...

@saltstack-formulas-travis
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noelmcloughlin @myii @saltstack-formulas-travis