Skip to content

Expanded registrations API. Added test. Updated docs.#2829

Merged
hueniverse merged 4 commits intohapijs:masterfrom
arb:registrations
Oct 9, 2015
Merged

Expanded registrations API. Added test. Updated docs.#2829
hueniverse merged 4 commits intohapijs:masterfrom
arb:registrations

Conversation

@arb
Copy link
Copy Markdown
Contributor

@arb arb commented Oct 6, 2015

Added more information to the plugin results of registrations.

@devinivy
Copy link
Copy Markdown
Member

devinivy commented Oct 6, 2015

Any reason the '0.0.0' default version is necessary?

@arb
Copy link
Copy Markdown
Contributor Author

arb commented Oct 6, 2015

Yes the 0.0.0 default is necessary. If a module doesn't have an attributes.pkg object or a attributes.version, then the plugin version ends up being undefined. Based on the tests and the default values in the plugin schema, I think the intent was to set the version to 0.0.0 in any instance where the version is not provided.

@hueniverse
Copy link
Copy Markdown
Contributor

hueniverse commented Oct 6, 2015

Hmm. The schema has the '0.0.0' default. Is it not working right?

@hueniverse hueniverse added the feature New functionality or improvement label Oct 6, 2015
@hueniverse hueniverse self-assigned this Oct 6, 2015
AdriVanHoudt
AdriVanHoudt reviewed Oct 6, 2015
View reviewed changes
lib/plugin.js
Copy link
Copy Markdown
Contributor

@AdriVanHoudt AdriVanHoudt Oct 6, 2015

Choose a reason for hiding this comment

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

why not just item as a whole?

@arb
Copy link
Copy Markdown
Contributor Author

arb commented Oct 6, 2015

@hueniverse it works right if your plugin has register.attributes.pkg = {}, the the Joi default on "version" pops and defaults it to "0.0.0". In instances where you don't have anything on pkg, only the default value of {} is set to pkg; not on pkg.version.

@hueniverse
Copy link
Copy Markdown
Contributor

hueniverse commented Oct 6, 2015

The we need to fix the schema, not the code.

@arb arb mentioned this pull request Oct 6, 2015
Closed
@arb
Copy link
Copy Markdown
Contributor Author

arb commented Oct 7, 2015

@hueniverse updated schema so we could remove the || '0.0.0' code change.

hueniverse
hueniverse reviewed Oct 7, 2015
View reviewed changes
lib/plugin.js Outdated
Copy link
Copy Markdown
Contributor

@hueniverse hueniverse Oct 7, 2015

Choose a reason for hiding this comment

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

This is redundant given you have attributes.

@arb
Copy link
Copy Markdown
Contributor Author

arb commented Oct 9, 2015

This should be good for a second review.

@hueniverse hueniverse added this to the 10.4.2 milestone Oct 9, 2015
hueniverse added a commit that referenced this pull request Oct 9, 2015
@hueniverse
Merge pull request #2829 from arb/registrations
03790cc 
Expanded `registrations` API. Added test. Updated docs.
@hueniverse hueniverse merged commit 03790cc into hapijs:master Oct 9, 2015
@arb arb deleted the registrations branch October 9, 2015 20:17
@lock
Copy link
Copy Markdown

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@hueniverse hueniverse

Labels

feature New functionality or improvement

Projects

None yet

Milestone

10.5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@arb @devinivy @hueniverse @AdriVanHoudt