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

Preparation for Node 14 #49

Closed
wants to merge 8 commits into from

Conversation

duncdrum
Copy link

@duncdrum duncdrum commented Jul 30, 2021

Chores

This includes a number of dependency updates, and config changes to enable building under node 14.
The source of the max-version is TypeFox/find-git-exec#22 so to enable building and running tests on node 13+ i changed the install command on CI, and added additional node versions on there as wel. (These won't run with this PR though since only master is configured to run CI. See #50

close #48

Tests

The test failures, see #46, are present irrespective of node version. From my local testing it appears that the code behaves identical between master and here.

Further changes to tests are necessary. I need some input on how you want to deal with these.

Code changes:

compatibility updates for latest version of deps. Added null check, and changed deprecated mocha.opts

Documentation:

some edits to the readme
close #47

see #43

duncdrum added a commit to evolvedbinary/fusion-studio that referenced this pull request Jul 30, 2021
see eclipse-theia/dugite-extra#49 we can yolo this one

adjust documenation

see #93
see #91
see #94
@msujew
Copy link
Member

msujew commented Aug 6, 2021

@duncdrum I'm in the process of updating find-git-exec, see TypeFox/find-git-exec#23. Expect a 0.0.4 release next week which you can include in your changes.

@msujew
Copy link
Member

msujew commented Aug 20, 2021

@duncdrum Version 0.0.4 of find-git-exec is now online. Would you mind updating your PR?

Comment on lines +47 to +51
"concurrently": "^6.2.0",
"cross-env": "^7.0.3",
"fs-extra": "^10.0.0",
"mocha": "^9.0.3",
"node-ssh": "^12.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

The testing failures are due to these dev dependencies. E.g. mocha@^9 needs Node 12. Please revert the dev dependencies to the previous versions (or at least versions compatible with Node 10).

Copy link
Author

Choose a reason for hiding this comment

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

please see my comments in #46 I don't see how downgrading mocha could possibly solve the fact that the default git branch name is main on new machines, while the tests hardcode master. This means tests will fail when run locally. I'd rather fix these 4 failing tests by addressing the underlying issues, before pinning an outdated mocha version.

Copy link
Member

Choose a reason for hiding this comment

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

I see, makes sense 👍

@msujew
Copy link
Member

msujew commented Sep 4, 2021

@duncdrum Are you still interested in contributing these changes? Please upgrade find-git-exec to 0.0.4. If not, I would like to take over the upgrading process.

@duncdrum
Copy link
Author

duncdrum commented Sep 7, 2021

@msujew i m back from my vacation and will update find-git-exec shortly, I m still not sure how the maintainers would like to address the 4 failing tests.

@anavarre
Copy link

Is #54 a duplicate? I'm seeing the EOL for Node 12 is in early Q2 2022 so are there any plans to get this issue fixed in Q1?

@msujew
Copy link
Member

msujew commented Feb 8, 2022

Closed in favor of #54

@msujew msujew closed this Feb 8, 2022
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.

ci: add forward looking test document minimum git version requirement
3 participants