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

feat(options): add support for different manifest input than bundle.dest #2

Merged
merged 4 commits into from
Feb 7, 2017

Conversation

msc117
Copy link
Contributor

@msc117 msc117 commented Feb 6, 2017

No description provided.

@phamann
Copy link
Owner

phamann commented Feb 7, 2017

Hey @msc117 👋 thanks for the PR. Can you describe a scenario when someone might need to use a different key in the manifest other than the original bundle destination?

@msc117
Copy link
Contributor Author

msc117 commented Feb 7, 2017

In my use case, I'm using path.resolve for bundle.dest. This causes the manifest to include the full file path as the input, i.e. /Users/msc117/Projects/... which is not ideal for my project.

Copy link
Owner

@phamann phamann left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me. Only minor point is changing the property name to something more specific.

README.md Outdated
@@ -85,6 +85,15 @@ Example manifest:
}
```

### input
Copy link
Owner

Choose a reason for hiding this comment

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

As input is quite an ambiguous property name, especially in the context of Rollup. Can you change this to something more specific, such as manifestInput, manifestKey or manifestProperty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to switch it to manifestKey

Copy link
Owner

Choose a reason for hiding this comment

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

👍

README.md Outdated
Default: `bundle.dest`
Required: `false`

The name of your filename intro used in creating manifest.
Copy link
Owner

Choose a reason for hiding this comment

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

Change to something along the lines of: "The filename used as the input key in the generated manifest map."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@phamann
Copy link
Owner

phamann commented Feb 7, 2017

Thanks for your help!

@phamann phamann merged commit 1debe22 into phamann:master Feb 7, 2017
@phamann
Copy link
Owner

phamann commented Feb 7, 2017

I've now published this as v.1.2.0

@msc117
Copy link
Contributor Author

msc117 commented Feb 7, 2017

Thanks!

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.

None yet

2 participants