Skip to content

Remove AIX support again#25

Merged
sindresorhus merged 3 commits into
masterfrom
remove-aix
Feb 23, 2019
Merged

Remove AIX support again#25
sindresorhus merged 3 commits into
masterfrom
remove-aix

Conversation

@silverwind

@silverwind silverwind commented Feb 22, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

See silverwind/default-gateway#10. In summary, I had a os-locked optional native dependency to support AIX but it turns out npm has a bug where it will ignore os in some cases, leading to gyp error spam on commands like npm ci and npm install --force.

Dropped the platform again for this reason until someone provides a pure-js implementation. Ideally, this should be released as semver-major.

sindresorhus
sindresorhus reviewed Feb 22, 2019
View reviewed changes
Comment thread package.json Outdated
"openbsd",
"sunos",
"win32"
],

@sindresorhus sindresorhus Feb 22, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't use os in any packages. It's buggy and I don't want to prevent a consumer of this package to support different platforms just because they depend on internal-ip. Runtime checks are better with optionalDependencies is a better choice.

But it doesn't matter. default-gateway has os, so this package doesn't need it anyway.

@silverwind silverwind Feb 23, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, os removed.

@sindresorhus sindresorhus changed the title remove AIX support again Remove AIX support again Feb 22, 2019
@silverwind

silverwind commented Feb 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Went ahead and removed os in default-gateway too. It's better that way because there is no issue with installing on unsupported platforms like AIX. It will allow dependants to gracefully treat the thrown Unsupported Platform error which is better than failing the installation entirely.

This was referenced Feb 23, 2019
Closed
Closed
@sindresorhus

sindresorhus commented Feb 23, 2019

Copy link
Copy Markdown
Owner

Will this require a major release of internal-ip?

@silverwind

silverwind commented Feb 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

It is breaking only for AIX users which should represent like 0.00001% of the users (if any at all). Your decision.

@sindresorhus sindresorhus merged commit cbb0a1e into master Feb 23, 2019
@sindresorhus sindresorhus deleted the remove-aix branch February 23, 2019 13:09
@arthureggert arthureggert mentioned this pull request Feb 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sindresorhus sindresorhus sindresorhus left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@silverwind @sindresorhus