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.

Sign up for GitHub

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

Jump to bottom

Update code style and dependencies #498

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update code style and dependencies #498

wants to merge 2 commits into from

Conversation

luislobo
Copy link
Contributor

@luislobo luislobo commented May 20, 2023 •
edited

Summary

  • Update URLs in .editorconfig and .jshintrc files
  • Add new rules to .eslintrc file for code style consistency
  • Remove .jshintrc file
  • Tests run on Ubuntu 20.04 and Windows: Node 16, 18, 19 and 20, with MongoDB 4.4.22 and 5.0.18

Details

The URLs in the .editorconfig and .jshintrc files were updated to use HTTPS instead of HTTP. In addition, the .eslintrc file was updated with new rules for code style consistency, including requiring let or const instead of var, enforcing spacing before blocks and functions, requiring parens in arrow function arguments, using template literals instead of string concatenation, and more.

The .jshintrc file was removed as it is no longer needed. Finally, the node.js versions in the .travis.yml file were updated to include newer versions.

@luislobo
Update code style and dependencies
a1af638 
## Summary
- Update URLs in .editorconfig and .jshintrc files
- Add new rules to .eslintrc file for code style consistency
- Remove .jshintrc file
- Update node.js versions in .travis.yml file

## Details
The URLs in the `.editorconfig` and `.jshintrc` files were updated to use HTTPS instead of HTTP. In addition, the `.eslintrc` file was updated with new rules for code style consistency, including requiring `let` or `const` instead of `var`, enforcing spacing before blocks and functions, requiring parens in arrow function arguments, using template literals instead of string concatenation, and more.

The `.jshintrc` file was removed as it is no longer needed. Finally, the node.js versions in the `.travis.yml` file were updated to include newer versions.

## Statistics
- 5 files changed, 1 deleted 
- 23 insertions(+), 141 deletions(-)

No dependency updates found.
@sailsbot
Copy link
Collaborator

Thanks for submitting this pull request, @luislobo! We'll look at it ASAP.

In the mean time, here are some ways you can help speed things along:

  • discuss this pull request with other contributors and get their feedback. (Reactions and comments can help us make better decisions, anticipate compatibility problems, and prevent bugs.)
  • ask another JavaScript developer to review the files changed in this pull request. (Peer reviews definitely don't guarantee perfection, but they help catch mistakes and enourage collaborative thinking. Code reviews are so useful that some open source projects require a minimum number of reviews before even considering a merge!)
  • if appropriate, ask your business to sponsor your pull request. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
  • make sure you've answered the "why?" (Before we can review and merge a pull request, we feel it is important to fully understand the use case: the human reason these changes are important for you, your team, or your organization.)

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

@luislobo luislobo force-pushed the update-eslint-and-code-based-on-changes branch 3 times, most recently from 6ea76f9 to 734b772 Compare May 22, 2023 06:49
@Josebaseba
Copy link
Contributor

We should merge this PR and then upgrade the mongo driver to the latest version. We should have mongo 5, 6 and 7 working properly.

@luislobo can you give us some updates? Where should we focus?

@luislobo
Copy link
Contributor Author

I can work on this during these days. I'm really short on time but can get it working. I don't think it's worth testing on AppVeyor.

I can test on the newest drivers too.

@Josebaseba
Copy link
Contributor

I don't mind working on the new drivers implementation.

But your PR is great, so I think that the best way here is, first get this PR merged and then implement the new drivers on top of your work.

If you want to add the new drivers in this current PR that would be awesome too.

@luislobo
Copy link
Contributor Author

I'd prefer to do it on a separate one as this one is focused on CI/CD, test updates and code cleanup.

@mikermcneil @eashaw How do you want to move forward on this module maintenance?

@luislobo luislobo force-pushed the update-eslint-and-code-based-on-changes branch from 84a9c3c to 89a4799 Compare October 1, 2023 04:18
@luislobo luislobo force-pushed the update-eslint-and-code-based-on-changes branch from 89a4799 to ee715f6 Compare October 1, 2023 04:18
@luislobo luislobo mentioned this pull request Oct 1, 2023
Merged
@luislobo
Copy link
Contributor Author

luislobo commented Oct 5, 2023 •
edited

@Josebaseba I've been helping @DominusKelvin on MongoDB driver updates and some of the var -> let/const conversions.

Can you please take a look at that PR?

#499

The plan is to move that one forward.

@DominusKelvin
Copy link
Collaborator

Yes @Josebaseba

The PR mentioned will automatically have most of the changes that this PR would have added to sails-mongo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
4 participants
@luislobo @sailsbot @Josebaseba @DominusKelvin