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(yaml): add documentMode, safe options #137

Merged
merged 6 commits into from Jan 6, 2020
Merged

Conversation

@shellscape
Copy link
Collaborator

shellscape commented Jan 3, 2020

Rollup Plugin Name: yaml

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

Fixes #39. This PR adds support for multiple document streams in a single YAML file, as well as the ability to load YAML using safeLoad and safeLoadAll, rather than the default load.

/cc @lufego

@shellscape shellscape requested review from pnevares, NotWoods, TrySound and rollup/plugin-maintainers Jan 3, 2020
@shellscape shellscape self-assigned this Jan 3, 2020
@shellscape shellscape force-pushed the feat/yaml/safeload branch 3 times, most recently from eb8b3be to c0f7e63 Jan 3, 2020
@shellscape shellscape force-pushed the feat/yaml/safeload branch from c0f7e63 to 2ddc5cd Jan 3, 2020
@shellscape

This comment has been minimized.

Copy link
Collaborator Author

shellscape commented Jan 3, 2020

Whew finally got this one sorted. It seems that the containers that Github Actions uses has changed somewhat in the last week, and directory paths are now windows standard paths and had to be escaped.

@shellscape shellscape mentioned this pull request Jan 3, 2020
3 of 9 tasks complete
shellscape added 2 commits Jan 5, 2020
@shellscape

This comment has been minimized.

Copy link
Collaborator Author

shellscape commented Jan 5, 2020

24 hours before I merge this badboy. Please review!

@Kocal
Kocal approved these changes Jan 6, 2020
const ext = /\.ya?ml$/;

export default function yamll(options = {}) {
export default function yamll(opts = {}) {

This comment has been minimized.

Copy link
@NotWoods

NotWoods Jan 6, 2020

Member

Since opts is just used with Object.assign we can remove the default value.

Suggested change
export default function yamll(opts = {}) {
export default function yamll(opts) {

This comment has been minimized.

Copy link
@shellscape

shellscape Jan 6, 2020

Author Collaborator

This is a stylistic choice. We don't lose anything by defining the default value there. It'll be moot when this is moved to TS.

@shellscape shellscape merged commit 973e05a into master Jan 6, 2020
9 checks passed
9 checks passed
10 (Windows)
Details
10 (Windows)
Details
8 (Windows)
Details
8 (Windows)
Details
ci/circleci: analysis Your tests passed on CircleCI!
Details
ci/circleci: node-v10-latest Your tests passed on CircleCI!
Details
ci/circleci: node-v12-latest Your tests passed on CircleCI!
Details
ci/circleci: node-v8-latest Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing d2daede...27bae28
Details
@shellscape shellscape deleted the feat/yaml/safeload branch Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.