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

refactor(core): Push validation logic to base class #1108

Merged
merged 3 commits into from
Nov 27, 2018
Merged

refactor(core): Push validation logic to base class #1108

merged 3 commits into from
Nov 27, 2018

Conversation

ezimanyi
Copy link
Contributor

A few changes to reduce the amount of code needed to add a new halyard config parameter; details on each commit message.

All subclasses of Node currently implement accept identically;
push the implementation to the base class to remove a lot of
duplicated code.
In most cases, we want the children of a given node to be
any declared fields that are also nodes; many nodes currently
implement getChildren identically via a call to makeReflectiveIterator.
Even in cases where getChildren is defined as an empty iterator or
as an explicit iteration, it is usually just an explicit expression
of what makeReflectiveIterator would produce.

This change implements getChildren in the base class as a call to
makeReflectiveIterator. It does not remove overrides that explicitly
define their own iterator (even if that iterator is identical to that
generated by makeReflectiveIterator).
Copy link
Member

@lwander lwander left a comment

Choose a reason for hiding this comment

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

Nice~!

@ezimanyi ezimanyi merged commit 8bd9e3e into spinnaker:master Nov 27, 2018
@ezimanyi ezimanyi deleted the refactor-validation branch November 27, 2018 21:07
kevinawoo added a commit to armory-io/halyard that referenced this pull request Nov 30, 2018
* master:
  feat(deploy/kubernetes): custom volume mnt per svc (spinnaker#1110)
  refactor(core): Push validation logic to base class (spinnaker#1108)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants