feat: update to js-yaml v4, reduce bundle size by ~70% #145

Open

sakulstra wants to merge 4 commits from sakulstra/fix/adjust-missing-files into master

sakulstra commented

2022-06-20 22:03:13 +00:00

(Migrated from github.com)

it's https://github.com/jonschlinkert/gray-matter/pull/137 + the adjusted missing files from the comment

@jonschlinkert would appreciate if you could have a look, the change seems trivial and tests are still passing

it's https://github.com/jonschlinkert/gray-matter/pull/137 + the adjusted missing files from the comment @jonschlinkert would appreciate if you could have a look, the change seems trivial and tests are still passing

👍 1

jonschlinkert (Migrated from github.com) reviewed

2022-06-22 13:26:16 +00:00

jonschlinkert (Migrated from github.com) left a comment

@sakulstra thanks for the PR.

If you could make the suggested change that would be great. FWIW I was planning on switching to this yaml lib, since it's lighter, well tested, and the code is pretty solid. Since we're doing a major version bump anyway, would you mind trying out that lib instead? And if all tests pass, let's use that library. If not we stick with yaml-js for now until someone has a chance to look into it.

Either way, thanks for taking the time to submit this PR.

@sakulstra thanks for the PR. If you could make the suggested change that would be great. FWIW I was planning on switching to [this yaml lib](https://www.npmjs.com/package/yaml), since it's lighter, well tested, and the code is pretty solid. Since we're doing a major version bump anyway, would you mind trying out that lib instead? And if all tests pass, let's use that library. If not we stick with yaml-js for now until someone has a chance to look into it. Either way, thanks for taking the time to submit this PR.

package.json

Please revert this. version changes are typically done by maintainers. Also, I personally like to play it safe with major version bumps, especially since incompatible node versions can easily sneak in.

sakulstra (Migrated from github.com) reviewed

2022-06-23 07:35:53 +00:00

package.json

kk reverted that change 👍

kk reverted that change :+1:

sakulstra commented

2022-06-23 07:36:24 +00:00

(Migrated from github.com)

If you could make the suggested change that would be great. FWIW I was planning on switching to this yaml lib, since it's lighter, well tested, and the code is pretty solid.

will do in another pr to not block this one

> If you could make the suggested change that would be great. FWIW I was planning on switching to [this yaml lib](https://www.npmjs.com/package/yaml), since it's lighter, well tested, and the code is pretty solid. will do in another pr to not block this one

sakulstra commented

2022-07-01 08:29:53 +00:00

(Migrated from github.com)

@jonschlinkert any chance this could be merged?

👍 2

This pull request has changes conflicting with the target branch.

lib/engines.js
package.json
test/parse-custom.js

View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Merge commit title

Merge pull request 'feat: update to js-yaml v4, reduce bundle size by ~70%' (#145) from sakulstra/fix/adjust-missing-files into master

Merge commit body

Reviewed-on: https://repo.reapertech.com/rtlabs-llm-agents/gms-gray-matter/pulls/145

Merge pull request 'feat: update to js-yaml v4, reduce bundle size by ~70%' (#145) from sakulstra/fix/adjust-missing-files into master

Reviewed-on: https://repo.reapertech.com/rtlabs-llm-agents/gms-gray-matter/pulls/145