Skip to content

Conversation

@lukasholzer
Copy link
Contributor

@lukasholzer lukasholzer commented Aug 7, 2023

🎉 Thanks for submitting a pull request! 🎉

Summary

Is part of fixing: https://github.com/netlify/pod-dev-foundations/issues/571

To be able to detect _headers or _redirects for a package when working from the repository root we need an additional field called package_path this will be set via the site picker on the React UI and persisted on the API.

By providing this to netlify config we can still separate the execution root from the location where we store and retrieve the configuration.

The implementation depends on this conversation here: https://netlify.slack.com/archives/C05556LEX28/p1691423437855069


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)

@lukasholzer lukasholzer requested a review from a team as a code owner August 7, 2023 16:31
@lukasholzer lukasholzer self-assigned this Aug 7, 2023
@lukasholzer lukasholzer requested a review from a team as a code owner August 7, 2023 16:31
@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

build: {
environment: {},
publish: '.',
publish: packagePath || '.',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for Option A inside the Slack discussion: https://netlify.slack.com/archives/C05556LEX28/p1691423437855069

const path = originalPath.replace(LEADING_SLASH_REGEXP, '')
// TODO: Based on survey outcome: https://netlify.slack.com/archives/C05556LEX28/p1691423437855069
// If we like option B then:
// const pathA = resolve(baseRel, packagePath || '', path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be Option B in the discussion: https://netlify.slack.com/archives/C05556LEX28/p1691423437855069

eduardoboucas
eduardoboucas previously approved these changes Aug 7, 2023
eduardoboucas
eduardoboucas previously approved these changes Aug 7, 2023
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.

🚀

@lukasholzer lukasholzer enabled auto-merge (squash) August 7, 2023 18:18
@lukasholzer lukasholzer merged commit 4a62bac into main Aug 8, 2023
@lukasholzer lukasholzer deleted the feat/add-package-path-for-config branch August 8, 2023 11:05
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.

3 participants