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

Major update proposal #20

Open
6 of 11 tasks
brody4hire opened this issue Dec 5, 2018 · 7 comments
Open
6 of 11 tasks

Major update proposal #20

brody4hire opened this issue Dec 5, 2018 · 7 comments

Comments

@brody4hire
Copy link

brody4hire commented Dec 5, 2018

We would need to increase the major version number to avoid breaking dependents that may need to support the older Node.js versions.

I would like to do the following before starting the major update:

  • uuid@3 update
  • introduce eslint that can be run as an npm script
  • cleanup major issues found by eslint
  • other possible cleanup that may be triggered by changes to resolve eslint issues
  • optional: it would also be ideal if we could port to a more currently maintained testing framework as well
  • publish minor 1.1.0 update
@n1ru4l
Copy link
Contributor

n1ru4l commented Dec 7, 2018

Object.assign is available since 4.9.1 and most projects that are using cordova-node-xcode require the minimum node version6/8 as a dependency.

Also node versions prior to 6 are afaik no longer maintained (https://github.com/nodejs/Release).

I think we should certainly start modernizing this code!

@brody4hire
Copy link
Author

I hope to do this soon, but carefully to avoid breaking any users that may be using some of the older Node.js versions. We need to increase the major package version number whenever we drop one or more Node.js versions according to semver etiquette.

I do think you are right that not many app developers would be using Node.js pre-6.0 and version 6 is approaching EOL.

@brody4hire
Copy link
Author

I would like to get PR #24 and ideally PR #12 finished, to be published in a 1.0.1 release before starting the major updates. @n1ru4l I think it would be helpful if you could take a look at PR #12, to help determine whether or not we should merge it before publishing the next release.

@n1ru4l
Copy link
Contributor

n1ru4l commented Dec 9, 2018

Also since nodeunit seems deprecated (https://www.npmjs.com/package/nodeunit#deprecated-project) and from my perspective the current tests are a mess. E.g. adding my test for (#24) was frustrating since there was no nice way to see the actual differences of expected and actual for long strings.

Therefore I would recommend to update to a more mature testing framework. I personally would prefer jest (https://jestjs.io/).

Furthermore tools like prettier and eslint could improve the overall code quality/consistency. I have set those up for multiple projects and could do so for this one too!

@brody4hire
Copy link
Author

Therefore I would recommend to update to a more mature testing framework.

Agreed; I would rather discuss this in a separate issue.

I think it would make sense but not be absolutely necessary to do this after we drop deprecated Node.js version.

@fbartho
Copy link
Contributor

fbartho commented Mar 4, 2019

Could we possibly either convert this to TypeScript or start shipping TypeScript Definitions?

This would be super straightforward with the move to ES6 classes #38, as it would be nearly self-documenting!

@Esemesek
Copy link

We are converting https://github.com/react-native-community/cli to TypeScript and it would be awesome if this package had some kind of TS types: shipped with the package or hosted in DefinitelyTyped repository.

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

No branches or pull requests

4 participants