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

refactor: Lint project and remove deprecated code #43

Merged
merged 45 commits into from
Feb 15, 2024

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Jun 18, 2023

Pull Request

Issue

The project isn't linted

Closes: #44

Approach

  • Lint the project
  • Add babel dev dependencies
  • Update CI to test for successful lint
  • Replace deprecated substr with substring
  • Refactor rotateEncryptionKey() to use async/await instead of promise
  • Let dependabot update action versions weekly

Tasks

  • Ensure tests pass
  • Change repo settings to require new tests

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jun 18, 2023

Thanks for opening this pull request!

@cbaker6 cbaker6 marked this pull request as draft June 18, 2023 16:20
@codecov
Copy link

codecov bot commented Jun 18, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (09180e8) 90.83% compared to head (01c7283) 93.44%.
Report is 1 commits behind head on main.

Files Patch % Lines
index.js 97.36% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
+ Coverage   90.83%   93.44%   +2.60%     
==========================================
  Files           1        1              
  Lines         131      122       -9     
==========================================
- Hits          119      114       -5     
+ Misses         12        8       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cbaker6 cbaker6 marked this pull request as ready for review June 18, 2023 17:43
@cbaker6
Copy link
Contributor Author

cbaker6 commented Jun 18, 2023

@mtrezza @dblythy Can either revert semantic-release as suggested in #45 or merge this PR which currently uses the previous version of semantic-release to pass the CI

In the repo settings you should add protections for the following passing tests:

  • Lint
  • Node 14
  • Node 16
  • Node 18
  • Node 19

Remove the following as they are replaced by the aforementioned tests:

  • test (14)
  • test (16)
  • test (18)

@mtrezza
Copy link
Member

mtrezza commented Jun 18, 2023

Could you please resolve any conflicts; and keep the semantic release version of the main branch, it should work with #46.

@mtrezza mtrezza changed the title refactor: Lint project refactor: Add linting to repository Jun 18, 2023
@cbaker6
Copy link
Contributor Author

cbaker6 commented Jun 18, 2023

My reason for opening for #45 is because it results in the following when trying to install on a local machine:

npm WARN old lockfile 
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile 
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile 
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @parse/[email protected]
npm WARN Found: [email protected]
npm WARN node_modules/semantic-release
npm WARN   peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN   6 more (@semantic-release/changelog, @semantic-release/npm, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN node_modules/@semantic-release/commit-analyzer
npm WARN   @semantic-release/commit-analyzer@"^8.0.0" from [email protected]
npm WARN   node_modules/semantic-release
npm WARN   1 more (the root project)
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @parse/[email protected]
npm WARN Found: [email protected]
npm WARN node_modules/semantic-release
npm WARN   peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN   6 more (@semantic-release/changelog, @semantic-release/npm, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer semantic-release@">=15.8.0 <18.0.0" from @semantic-release/[email protected]
npm WARN node_modules/@semantic-release/changelog
npm WARN   dev @semantic-release/changelog@"5.0.1" from the root project
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @parse/[email protected]
npm WARN Found: [email protected]
npm WARN node_modules/semantic-release
npm WARN   peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN   6 more (@semantic-release/changelog, @semantic-release/npm, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN node_modules/@semantic-release/npm
npm WARN   @semantic-release/npm@"^7.0.0" from [email protected]
npm WARN   1 more (the root project)
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @parse/[email protected]
npm WARN Found: [email protected]
npm WARN node_modules/semantic-release
npm WARN   peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN   6 more (@semantic-release/changelog, @semantic-release/npm, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN node_modules/@semantic-release/git
npm WARN   dev @semantic-release/git@"9.0.0" from the root project
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @parse/[email protected]
npm WARN Found: [email protected]
npm WARN node_modules/semantic-release
npm WARN   peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN   6 more (@semantic-release/changelog, @semantic-release/npm, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN node_modules/@semantic-release/github
npm WARN   @semantic-release/github@"^7.0.0" from [email protected]
npm WARN   1 more (the root project)
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @parse/[email protected]
npm WARN Found: [email protected]
npm WARN node_modules/semantic-release
npm WARN   peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm WARN   6 more (@semantic-release/changelog, @semantic-release/npm, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer semantic-release@">=15.8.0 <18.0.0" from @semantic-release/[email protected]
npm WARN node_modules/@semantic-release/release-notes-generator
npm WARN   @semantic-release/release-notes-generator@"^9.0.0" from [email protected]
npm WARN   1 more (the root project)
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: @parse/[email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/semantic-release
npm ERR!   dev semantic-release@"21.0.1" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected]
npm ERR! node_modules/@semantic-release/commit-analyzer
npm ERR!   dev @semantic-release/commit-analyzer@"8.0.1" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

If you know of a better way without force installing, let me know, but the errors seem to point to wanting newer versions than what's in the package.json and package-lock

@mtrezza
Copy link
Member

mtrezza commented Jun 18, 2023

What Node and npm version are you running? I tried with Node 16 and 18 and it worked fine. It really just needs to run in our CI, and that's it. This release framework is our internal tool. Check here:

https://github.com/parse-community/parse-server-fs-adapter/actions/runs/5305463515/jobs/9602438824

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jun 18, 2023

I'm running 18.16.0. I tried installing using npm i -E [email protected]

@mtrezza
Copy link
Member

mtrezza commented Jun 18, 2023

The main branch works it seems, so why are you trying to install the dependency explicitly? If you use the main branch and then run npm i it should work I guess.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jun 18, 2023

It appears to me that dependabot allowed an incorrect update in #41 and somehow forced an update to the package-lock. The warnings and errors in #43 (comment) look explicit and state a number of problems like peer semantic-release@">=16.0.0 <18.0.0" from @semantic-release/[email protected].

#41 updated from [email protected] to [email protected] which are outside the peer dependency windows which is what I mentioned in #45. It seems if that update was to be done properly, other semantic-release updates should have been done at the same time.

In addition, the parse-server receives way more updates than this repo and it's versions aren't this high: https://github.com/parse-community/parse-server/blob/9674d4a2c0a9d0cda112056a6a2b1629931f37a3/package.json#L75-L107

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jun 18, 2023

The main branch works it seems, so why are you trying to install the dependency explicitly?

This PR requires babel/lint to be installed, installing using npm i will either revert semantic-release version to a compatible one (the current commit of this PR) or result in the errors mentioned in #43 (comment). I suspect the reasons for the this is what I mentioned in #43 (comment)

package.json Outdated Show resolved Hide resolved
@cbaker6 cbaker6 changed the title ci: Add linting to repository ci: Lint project and test node 19 Jan 22, 2024
@cbaker6
Copy link
Contributor Author

cbaker6 commented Jan 22, 2024

Ready for review...

In the repo settings you should add protections for the following passing tests:

  • ci/Node 14
  • ci/Node 16
  • ci/Node 18

Remove the following as they are replaced by the aforementioned tests:

  • test (14)
  • test (16)
  • test (18)

@cbaker6 cbaker6 changed the title ci: Lint project and test node 19 refactor: Lint project and remove deprecated code Jan 22, 2024
@mtrezza mtrezza merged commit 903a72d into parse-community:main Feb 15, 2024
7 checks passed
@cbaker6 cbaker6 deleted the lint branch February 15, 2024 14:21
@parseplatformorg
Copy link

🎉 This change has been released in version 3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project isn't linted
3 participants