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

Standardize test config, upgrade (some) devDeps, add named export #70

Merged
merged 11 commits into from
Feb 10, 2022

Conversation

agilgur5
Copy link
Owner

Summary

  • Standardizes test config, Travis config, npm scripts, gitignore etc
  • Upgrade and split out some devDeps (yes, some of these are once again outdated now)
  • Add a named export in addition to the default export

History

Basically all of these commits were authored 2 years ago (just rebased them, so commit time is different from authored time) while I was standardizing various aspects of my library set-up in several different repos I maintain (really need a codemod tool or something for this like mrm).

I never got to finishing it up or PR'ing it, and honestly I don't remember why. Usually it's because I get stuck on something, but I tried looking through my old code and honestly wasn't sure. I was very busy with a new job, the pandemic (Feb 2020), and maintaining TSDX, so might've just been a time constraint 🤷 . #42 was partially blocked on this as well.

The two most recent (tiny) docs commits I just made.

Changes

  • (refactor): move tests to test/, test-utils/ to test/config/
  • (env/refactor): standardize .gitignore
  • (deps): use jest-without-globals to import jest globals
  • (ci): upgrade to use Node v10
  • (deps): upgrade to canvas@2, jsdom@15
  • (pkg): add funding
  • (feat): add a named export (same as default)
  • (env): rename 'dist' script to 'build'
  • (deps): move the window.resizeTo polyfill to its own package
  • env: switch to main branch as default
  • ci: use travis-ci.com as .org is decomm'd

Review Notes

Yes, some of these deps are (once again) outdated now (e.g. Node 10 was EOL April 2021), but this is supposed to be merged before #42 which itself now has various out-of-date deps. So will wait for after/when that is merged to upgrade those.

- have been putting tests separately from source nowadays in other of
  my libraries, so standardize around that

- put fixtures just in test/ (later could be in test/fixtures/), leave
  config/ solely for test config type things
- basically the same one I use in all my other libraries
  - I believe it's an old version of Node output from gitignore.io iirc
- it has comments unlike the old one, so that's much nicer for those
  who aren't totally sure what each entry is for
- Node 8 was EOL in December, I upgraded locally a while ago too

- The default NPM version for Node v10 also supports `npm ci`,
  which should be a CI speed boost as well
- because canvas@1 is deprecated and is having install issues
  - see hhttps://travis-ci.org/agilgur5/react-signature-canvas/builds/650962136,
    https://travis-ci.org/agilgur5/react-signature-canvas/builds/658062587,
    etc
- because many folks prefer not to use default exports and because
  named exports are more explicit, etc, etc
- 'build' is a lot more intuitive and matches other usage
- now that GH officially supports branch renames, moving all my repos
  to use `main`
  - GH also creates redirects now, which seem to be working well for
    the badges right now, but better to move all of them anyway
  - also had to switch Codecov default branch etc
- https://blog.travis-ci.com/2021-05-07-orgshutdown

- made the switch over with my last PR, but needed to change the docs
  too
  - update badge to use `.com`
  - update link to use `.com`, `app.` subdomain, and `github/` URL
    that travis-ci.com currently uses

- also use `/branches` URL as the default URL shows current builds,
  which includes WIP PR/branch builds too
  - `/branches` shows the "default branch" first instead and various
    checkmarks etc, so it's a good summary page that can be drilled
    down from
@agilgur5 agilgur5 added kind: feature New feature or request kind: internal Changes only affect the internals and not the API or usage labels Feb 10, 2022
Copy link
Owner Author

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM. Tests pass locally.

Noting that this should be in a minor release as it adds a feature, the named export.

@agilgur5 agilgur5 added the version: minor Upgrade minor version when merged label Feb 10, 2022
@agilgur5
Copy link
Owner Author

Tests are passing on the CI branch build, but for some reason no PR build was triggered. Should be good to merge but I'll have to temporarily modify the status checks to do so (as I don't believe I can get Travis to re-trigger a PR build, only branches afaik)

@agilgur5 agilgur5 merged commit 2781869 into main Feb 10, 2022
@agilgur5 agilgur5 deleted the standardization branch February 10, 2022 18:29
Repository owner locked as resolved and limited conversation to collaborators Feb 11, 2022
@agilgur5 agilgur5 added the scope: tests Tests could be improved. Or changes that only affect tests label Jun 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: feature New feature or request kind: internal Changes only affect the internals and not the API or usage scope: tests Tests could be improved. Or changes that only affect tests version: minor Upgrade minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant