Skip to content

Conversation

@lukasholzer
Copy link
Contributor

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

Summary

Fixes #3854

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: bug code to address defects in shipped code label Dec 20, 2021
@github-actions
Copy link

github-actions bot commented Dec 20, 2021

πŸ“Š Benchmark results

Comparing with ecfc91f

Package size: 356 MB

(no change)

^  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  359 MB  356 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
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

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

If I understand correctly, this is doing the same check twice: once in the registry when creating the NetlifyFunction instance, and again in the constructor of NetlifyFunction. Is that necessary?

@lukasholzer
Copy link
Contributor Author

@eduardoboucas yes you are absolutely right this wasn't the most elegant approach πŸ˜‚ – I think I had to less coffee this morning (already updated)

eduardoboucas
eduardoboucas previously approved these changes Dec 20, 2021
Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

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

LGTM!

@lukasholzer
Copy link
Contributor Author

@eduardoboucas seems that some tests are failing need to investigate further maybe I broke something with the change

@lukasholzer lukasholzer added the automerge Add to Kodiak auto merge queue label Dec 20, 2021

async scan(directories) {
async scan(relativeDirs) {
const directories = relativeDirs.filter(Boolean).map((dir) => (isAbsolute(dir) ? dir : join(this.projectRoot, dir)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erezrokah @ehmicky I think this is the fastet way to fix it -> Wondering if we can fix it on the long term inside the netlify config.

Copy link
Contributor

@erezrokah erezrokah Dec 20, 2021

Choose a reason for hiding this comment

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

Yes, this should be addressed by netlify/build#3620.

@kodiakhq kodiakhq bot merged commit 93a1662 into main Jan 7, 2022
@kodiakhq kodiakhq bot deleted the fix/3854-fixes-an-issue-with-netlify-dev branch January 7, 2022 15:05
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: bug code to address defects in shipped code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Presence of a [dev] block in Netlify TOML causes local dev to fail for functions

4 participants