Skip to content

Meta object#13

Merged
delvedor merged 9 commits intomasterfrom
meta-object
Dec 18, 2017
Merged

Meta object#13
delvedor merged 9 commits intomasterfrom
meta-object

Conversation

@delvedor
Copy link
Member

First part of the work started from the discussion in fastify/fastify#456.
Once the core is ready we will merge and release this as well.

dependencies: {
fastify: ['plugin1', 'plugin2'],
reply: ['compress']
}
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit lost on how this dependencies block would be used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is still a WIP, checkout fastify/fastify#466.

const console = require('console')

function plugin (fn, version) {
function plugin (fn, options) {
Copy link
Member

Choose a reason for hiding this comment

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

I would keep it backward-compatible.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is

if (typeof options === 'string') {

Copy link
Member

Choose a reason for hiding this comment

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

this is not true anymore, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct.

README.md Outdated
`fastify-plugin` can do three things for you:
- Add the `skip-override` hidden property
- Check the bare-minimum version of Fastify
- Ass some custom meta to the plugin
Copy link
Member

Choose a reason for hiding this comment

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

How about we lose the "Ass" and go with this instead:

Asses plugin metadata

Copy link
Member Author

Choose a reason for hiding this comment

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

Oooops :D

README.md Outdated

You can check [here](https://github.com/npm/node-semver#ranges) how to define a `semver` range.

You can also pass some metadata thatr will be handled by Fastify, such as the dependencies of your plugin.
Copy link
Member

Choose a reason for hiding this comment

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

Strike "thatr" and replace with "that".

Copy link
Member

@allevo allevo left a comment

Choose a reason for hiding this comment

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

LGTM

test.js Outdated

try {
fp(() => {}, 12)
fp(() => {}, { version: 12 })
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep this test? Or this PR is a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

At the moment the second parameter should be a string or object.

@delvedor
Copy link
Member Author

Hello folks!
This is the last step before we release the next version of Fastify.
We are introducing a breaking change on how works this library.

From now we will accept only one option object with all the metadata inside, for example if you need to check the Fastify version you should pass an object like the following: { fastify: '>=0.x' }.
If you want to check the Fastify version, dependencies and declare a name:

{
  fastify: '>=0.x',
  name: 'cool-plugin',
  dependencies: {
    fastify: ['plugin1', 'plugin2'],
    reply: ['compress']
  },
  decorators: {
    reply: ['setCookie']
  }
}

@fastify/fastify do we all agree with this change?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

LGTM with a couple grammar items updated.

README.md Outdated
1. Use the `skip-override` hidden property
2. Use this module

In addition if you use this module when creating new plugins, you can declare the dependencies, the name and the expected Fastify version that your plugins needs.
Copy link
Member

Choose a reason for hiding this comment

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

plugins needs => plugin needs

README.md Outdated
- Add the `skip-override` hidden property
- Check the bare-minimum version of Fastify
- Pass some custom meta to the plugin
- Pass some custom meta of the plugin to Fastify
Copy link
Member

Choose a reason for hiding this comment

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

meta => metadata

@delvedor delvedor merged commit e37788b into master Dec 18, 2017
@delvedor delvedor deleted the meta-object branch December 18, 2017 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants