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

add sbt plugin #1962

Merged
merged 31 commits into from
May 8, 2021
Merged

add sbt plugin #1962

merged 31 commits into from
May 8, 2021

Conversation

laughedelic
Copy link
Contributor

@laughedelic laughedelic commented Apr 25, 2021

What Changed

Copy-pasted code from the git-tag, added relevant sbt commands (set version, publish, etc.), added canary hook.

Why

sbt is the de facto standard build tool for Scala projects.

I've tried to integrate with sbt through the exec plugin, it works perfectly for normal releases (only publish hook), but gets quite hairy for canaries:

  "plugins": [
    [
      "exec",
      {
        "publish": "sbt publish",
        "canary": "sbt --error --no-colors 'publish; set version/aggregate := false; print version' | tail -1"
      }
    ],
    "git-tag",
  ]

Mainly because canary combines publishing and versioning and requires new version as an output. And because sbt is not made for CLI scripts, so it's not easy to get the current version out of it.

So I took a stab at writing a proper plugin for sbt.

Todo:

I'm new to JavaScript/TypeScript, so take this with a grain of salt. If any code needs refactoring, I'll be grateful for pointers 🙏

  • Add docs
  • Add tests

I really have no idea how to test this 😥 especially with mocking and all. I've tested it manually on an actual project though.

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

🐤 Download canary assets:

auto-linux--canary.1962.23960.gz
auto-macos--canary.1962.23960.gz
auto-win.exe--canary.1962.23960.gz

📦 Published PR as canary version: under canary scope @[email protected]

✨ Test out this PR locally via:

npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
npm install @auto-canary/[email protected]
# or 
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]
yarn add @auto-canary/[email protected]

@laughedelic laughedelic changed the title add sbt plugin add sbt plugin (WIP) Apr 25, 2021
@adierkens adierkens added the minor Increment the minor version when merged label Apr 25, 2021
@laughedelic laughedelic changed the title add sbt plugin (WIP) add sbt plugin Apr 25, 2021
@laughedelic laughedelic marked this pull request as ready for review April 25, 2021 23:58
@codecov
Copy link

codecov bot commented Apr 25, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@3c51664). Click here to learn what that means.
The diff coverage is 79.06%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1962   +/-   ##
=======================================
  Coverage        ?   80.26%           
=======================================
  Files           ?       66           
  Lines           ?     5380           
  Branches        ?     1249           
=======================================
  Hits            ?     4318           
  Misses          ?      705           
  Partials        ?      357           
Impacted Files Coverage Δ
plugins/sbt/src/index.ts 79.06% <79.06%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c51664...7ef81a8. Read the comment docs.

@laughedelic
Copy link
Contributor Author

laughedelic commented Apr 26, 2021

@hipstersmoothie hi! thanks for publishing a canary release. I just tried installing it on CI and running auto shipit and got this error:

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /snapshot/auto/plugins/sbt/node_modules/strip-ansi/index.js
require() of ES modules is not supported.
require() of /snapshot/auto/plugins/sbt/node_modules/strip-ansi/index.js from /snapshot/auto/plugins/sbt/dist/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename /snapshot/auto/plugins/sbt/node_modules/strip-ansi/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /snapshot/auto/plugins/sbt/node_modules/strip-ansi/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1152:13)
    at Module.load (internal/modules/cjs/loader.js:984:32)
    at Function.Module._load (internal/modules/cjs/loader.js:877:14)
    at Module.require (internal/modules/cjs/loader.js:1024:19)
    at Module.require (pkg/prelude/bootstrap.js:1225:31)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.<anonymous> (/snapshot/auto/plugins/sbt/dist/index.js:7:46)
    at Module._compile (pkg/prelude/bootstrap.js:1320:22)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1156:10)
    at Module.load (internal/modules/cjs/loader.js:984:32) {
  code: 'ERR_REQUIRE_ESM'
}

in the config I refer to the plugin as just sbt.

@hipstersmoothie
Copy link
Collaborator

It looks like strip-ansi is now pure ESM. auto isn't set up to handle that yet so you'll have to use a 6.x version of strip-ansi

@laughedelic
Copy link
Contributor Author

Ok, done in bef0fe9 👌

@hipstersmoothie
Copy link
Collaborator

hipstersmoothie commented Apr 26, 2021

kicked off another canary for you

@laughedelic
Copy link
Contributor Author

Thanks! Just tried it out and it works as expected 🚀

Copy link
Collaborator

@hipstersmoothie hipstersmoothie left a comment

Choose a reason for hiding this comment

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

This is looking good so far! Will def want to add some tests. Those tests will require lots of mocking of the exec function. You should be able to looks at other plugins and emulate their testing setup. Mainly just want to make sure the logic works and your options do something.

@laughedelic
Copy link
Contributor Author

laughedelic commented Apr 27, 2021

@hipstersmoothie I wrote some tests. I haven't quite wrapped my head around mocking with Jest yet, so it's not very smooth. But it's better than nothing and actually tests some functionality/configuration. I would appreciate any pointers on how to improve it.

Copy link
Collaborator

@hipstersmoothie hipstersmoothie left a comment

Choose a reason for hiding this comment

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

Tests look great! Looks mostly like other tests, nothing to suggest really. Jest mocks can be tricky though, I agree

@hipstersmoothie
Copy link
Collaborator

Wait 1 more thing:

Add a link to the sbt plugin here and here

@laughedelic
Copy link
Contributor Author

@hipstersmoothie done: bd18523 👌

@laughedelic
Copy link
Contributor Author

@hipstersmoothie could you give it another look?

@hipstersmoothie hipstersmoothie merged commit df0218c into intuit:main May 8, 2021
@adierkens
Copy link
Collaborator

🚀 PR was released in v10.27.0 🚀

@adierkens adierkens added the released This issue/pull request has been released. label May 8, 2021
@laughedelic laughedelic deleted the feat/plugin/sbt branch May 9, 2021 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants