Skip to content

Conversation

hermannm
Copy link
Collaborator

@hermannm hermannm commented Nov 9, 2023

Background

While implementing #21, I noticed that the library appeared to have some redundant dependencies. As I like to avoid transitive dependencies where possible, I thought I'd propose these changes.

Changes

  • change IsColorTerminal to no longer depend on golang.org/x/term, since we already depend on go-isatty and can just use that
    • while doing this, I thought we might as well upgrade go-isatty to the newest version, which also lets us use a tagged version for golang.org/x/sys
  • add asciiValid and asciiValidPrint functions, to avoid the dependency on segmentio/encoding (outside of tests, but that doesn't affect transitive dependencies)
    • since the rest of the codebase is ported without depending on segmentio, I thought it was a shame that just these two functions brought that whole dependency
    • also ported the tests for these functions

Sorry if I should have raised an issue about this first, but hope you might appreciate it anyway!

Copy link
Owner

@neilotoole neilotoole left a comment

Choose a reason for hiding this comment

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

LGTM.

@neilotoole neilotoole merged commit ab3db80 into neilotoole:master Nov 10, 2023
@neilotoole
Copy link
Owner

@hermannm Thank you, this is a great improvement! It's greatly appreciated.

I've got a bit of work to do on the GH workflow, it hasn't been touched in a while and has deprecation warnings for old versions of actions, and then I'll tag and release.

Thank you for all the work. Do you have any interest in being added as a collaborator on the repo?

@hermannm
Copy link
Collaborator Author

Thank you for all the work. Do you have any interest in being added as a collaborator on the repo?

@neilotoole Sure, I'd be delighted to!

@neilotoole
Copy link
Owner

@hermannm Excellent. Invitation sent. Welcome!

Also: v0.7.0 has been tagged. Please feel free to verify if you have time.

@hermannm
Copy link
Collaborator Author

@neilotoole Great to hear! I'll check it out

@hermannm hermannm deleted the remove-dependencies branch November 10, 2023 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants