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

chore!: remove node 10 support #3648

Merged
merged 7 commits into from
Nov 18, 2021
Merged

chore!: remove node 10 support #3648

merged 7 commits into from
Nov 18, 2021

Conversation

lukasholzer
Copy link
Contributor

πŸŽ‰ Thanks for submitting a pull request! πŸŽ‰

Summary

Fixes #3512


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code πŸ§‘β€πŸ’». This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire πŸ”₯ (e.g. incident related), you can skip this step.
  • Read the contribution guidelines πŸ“–. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) πŸ§ͺ
  • Update or add documentation (if features were changed or added) πŸ“
  • Make sure the status checks below are successful βœ…

A picture of a cute animal (not mandatory, but encouraged)

image

@github-actions github-actions bot added the type: chore work needed to keep the product and development running smoothly label Nov 16, 2021
@github-actions
Copy link

github-actions bot commented Nov 16, 2021

πŸ“Š Benchmark results

Comparing with a875fb7

Package size: 364 MB

⬇️ 0.00% decrease vs. a875fb7

^  364 MB  364 MB  364 MB  364 MB  364 MB  364 MB  364 MB  364 MB  365 MB  365 MB  365 MB  365 MB  364 MB 
β”‚   β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

This is looking πŸ”₯ @lukasholzer, answered your question about Edge Handlers and added a comment.

tests/command.deploy.test.js Outdated Show resolved Hide resolved
tests/command.dev.test.js Outdated Show resolved Hide resolved
@ehmicky
Copy link
Contributor

ehmicky commented Nov 16, 2021

This is looking great to me too! πŸ‘
Nothing to add except for what @erezrokah mentioned, with one exception: the CI tests are currently failing, which might indicate some problem.

@lukasholzer
Copy link
Contributor Author

This is looking great to me too! πŸ‘ Nothing to add except for what @erezrokah mentioned, with one exception: the CI tests are currently failing, which might indicate some problem.

Yea currently figuring out why the tests are failing – therefore I set the PR in the draft state. This should indicate that it is not ready for review – I try to create always fast PR's so that folks that are interested can already see in which direction it turns, fast feedback.

But it is strange that it is failing as my changes should not influence that

opened up a issuei in the edge handler to drop support netlify-plugin-edge-handlers#660
@lukasholzer lukasholzer marked this pull request as ready for review November 17, 2021 11:09
@lukasholzer
Copy link
Contributor Author

The build (ubuntu-latest, 10.x) is hanging around as there is now a new Testing for CLI / build (ubuntu-latest, 12.x) that we need to add as required status checks once this PR is merged

@erezrokah
Copy link
Contributor

The build (ubuntu-latest, 10.x) is hanging around as there is now a new Testing for CLI / build (ubuntu-latest, 12.x) that we need to add as required status checks once this PR is merged

I updated the branch protections settings, so this should be fixed now

@erezrokah
Copy link
Contributor

I updated the branch protections settings, so this should be fixed now

You should be able to do it too I think as you're a part of https://github.com/orgs/netlify/teams/netlify-dev

Copy link
Contributor

@ehmicky ehmicky left a comment

Choose a reason for hiding this comment

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

πŸŽ‰

@lukasholzer lukasholzer added the automerge Add to Kodiak auto merge queue label Nov 18, 2021
@kodiakhq kodiakhq bot merged commit 86931c6 into main Nov 18, 2021
@kodiakhq kodiakhq bot deleted the chore/node-12-update branch November 18, 2021 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Add to Kodiak auto merge queue type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade minimal Node.js version to Node 12
3 participants