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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(formatters): move formatters to a separate package #2468

Merged
merged 7 commits into from
May 23, 2023

Conversation

sennyeya
Copy link
Contributor

Fixes #2467. Moves the formatters to their own package that is then imported by @stoplight/spectral-cli.

馃憖 @P0lip

Checklist

  • Tests added / updated
  • Docs added / updated

Existing tests should cover any issues, this PR is just a move with additional dependency work.

Does this PR introduce a breaking change?

  • Yes
  • No

The formatters for the CLI have been pulled out to their own package, not necessarily breaking for any contracts, but will require pulling in a new package on CLI version update.

Additional context

I wasn't sure if this should sit in packages/core or a separate package, I think it makes sense to pull it completely out as it's not core functionality and just a presentation layer, but I could go either way.

@sennyeya sennyeya requested review from a team as code owners May 15, 2023 14:03
@sennyeya sennyeya requested a review from pamgoodrich May 15, 2023 14:03
packages/formatters/README.md Outdated Show resolved Hide resolved
packages/formatters/README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@sennyeya sennyeya force-pushed the formatters branch 2 times, most recently from bcb7afb to eb169fc Compare May 15, 2023 15:33
@sennyeya
Copy link
Contributor Author

Couple of questions:

  1. What is the commit message supposed to look like? Not sure I'm following from the error message.
  2. Is there somewhere that the memfs is populated with the HTML files during Karma tests, getting a not found error locally and in the CI, but not sure where that would be solved?

@sennyeya sennyeya changed the title feat(formatters): Update where formatters are stored. feat(formatters): move formatters to a separate package May 15, 2023
@sennyeya
Copy link
Contributor Author

@P0lip Any ideas on why the Karma tests are failing? I think it's related to memfs mocks, but it seems odd that moving the directory causes this issue in the same test case.

@P0lip
Copy link
Contributor

P0lip commented May 17, 2023

Yeah, that is most likely due to the HTML formatter relying on fs and it not being available in Karma launcher we use (it's plain Chrome browser in headless mode, we use webpack to bundle things and have some basic fs mock).
I'll try to update HTML formatter to not depend on fs at all so that we don't need to worry about anything.

@P0lip
Copy link
Contributor

P0lip commented May 17, 2023

#2474 - WIP

@sennyeya
Copy link
Contributor Author

Great, thanks!

@P0lip
Copy link
Contributor

P0lip commented May 18, 2023

@sennyeya could you merge the latest changes when you have a minute? 馃檹 that should unblock test-browser 馃

@sennyeya sennyeya force-pushed the formatters branch 2 times, most recently from 0c14d35 to 8ffedc8 Compare May 18, 2023 20:04
@sennyeya
Copy link
Contributor Author

@P0lip It looks like cliui is having issues with the Karma browser tests, which I think is expected as it's just a nodejs module. I think this was excluded originally by including packages/ci/** in the karma.config.ts exclude section. As you'll be updating this package to be all environment friendly, I think it makes sense to skip this for now.

Other than that, everything should be good minus the commit message (what's the format for those?)

Signed-off-by: Aramis Sennyey <sennyeyaramis@gmail.com>
@P0lip
Copy link
Contributor

P0lip commented May 19, 2023

Other than that, everything should be good minus the commit message (what's the format for those?)

it has to be accepted by commitlint per its config here https://github.com/stoplightio/spectral/blob/develop/commitlint.config.js

@P0lip
Copy link
Contributor

P0lip commented May 19, 2023

I'll take a final look on Monday, and if it's all good I will proceed and merge it.
Thanks a lot of your time and contribution. Appreciate it a lot.

P0lip
P0lip previously approved these changes May 23, 2023
@P0lip P0lip merged commit 664e259 into stoplightio:develop May 23, 2023
8 checks passed
@sennyeya
Copy link
Contributor Author

@P0lip Thanks for your help pushing this through!

stoplight-bot pushed a commit that referenced this pull request May 23, 2023
# [6.7.0](v6.6.0...v6.7.0) (2023-05-23)

### Bug Fixes

* **core:** improve deep ruleset inheritance ([#2326](#2326)) ([378b4b8](378b4b8))
* **core:** more accurate ruleset error paths ([66b3ca7](66b3ca7))
* **core:** reset path in fn context ([#2389](#2389)) ([3d47ec4](3d47ec4))
* **ref-resolver:** bump @stoplight/json-ref-resolver from ~3.1.4 to ~3.1.5 ([#3635](https://github.com/stoplightio/spectral/issues/3635)) ([215ae93](215ae93))
* **ruleset-bundler:** defaults should be last one ([#2403](#2403)) ([8780cfa](8780cfa))
* **ruleset-bundler:** remove extraneous 'external dependency' warnings ([#2475](#2475)) ([e791534](e791534))
* **ruleset-migrator:** avoid positive lookbehinds ([#2349](#2349)) ([455c324](455c324))
* **ruleset-migrator:** transform functions under overrides ([#2459](#2459)) ([45e817f](45e817f))
* **ruleset-migrator:** use module for require.resolve ([#2405](#2405)) ([d7c0fa4](d7c0fa4))
* **rulesets:** avoid false errors from ajv ([#2408](#2408)) ([92dab78](92dab78))
* **rulesets:** length.min said "must not be longer than" ([#2355](#2355)) ([df3b6f9](df3b6f9))

### Features

* **core:** relax formats validation ([#2151](#2151)) ([de16b4c](de16b4c))
* **core:** support end-user extensions in the rule definitions ([#2345](#2345)) ([365fced](365fced))
* **core:** support x- extensions in the ruleset ([#2440](#2440)) ([964151e](964151e))
* **formats:** support AsyncAPI 2.6.0 ([#2391](#2391)) ([b8e51b4](b8e51b4))
* **formatters:** move formatters to a separate package ([#2468](#2468)) ([664e259](664e259))
* **rulesets:** add traits array path to headers rule ([#2460](#2460)) ([9ceabca](9ceabca))
* **rulesets:** support AsyncAPI 2.6.0 ([#2391](#2391)) ([94a7801](94a7801))
@stoplight-bot
Copy link
Collaborator

馃帀 This PR is included in version 6.7.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

@stoplight-bot
Copy link
Collaborator

馃帀 This PR is included in version 1.18.1 馃帀

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose formatters from either a separate package or the CLI package
3 participants