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

test: added a lot tests around the monorepo structure #166

Merged
merged 5 commits into from May 24, 2020

Conversation

Jefiozie
Copy link
Contributor

@Jefiozie Jefiozie commented Mar 21, 2020

Hi @kamilmysliwiec,

Here is my draft PR with an explanation of what I've changed and what the behavior is/ will be. I will split this in two different part, part one is for the default setup that is not a monorepo. The other one is specific for the monorepo setup.

Default setup

To use the library people do the following:

  1. nest new projectName
  2. nest add @nestjs/azure-func-http
  3. npm run start:azure

The output of this will be the same as earlier. In the dist folder will be all the code and because of the configuration in the Azure files it will be runnable by running func host start

No new things there, the only thing I would like to know. Should we support a nest.json file? This is something I've not yet implemented at this moment.

Monorepo setup

To use the library people do the following:

  1. nest new projectName
  2. nest generate app projectTwo
  3. Under the hood it will be transformed to a monorepo workspace
  4. nest add @nestjs/azure-func-http --project projectTwo
  5. nest build projectTwo
  6. Under the hood, we will publish in the dist/apps/projectTwo with a custom webpack setup
  7. cd apps/projectTwo/src
  8. npx func host start

The output of the dist will be runnable similiar as the Default Setup

Could you provide me some feedback and the answer if we need to support nest.json. I don't know if the custom webpack setup is the best way to go as this will break the default behavior of nest start projectTwo. But I don't see a other way we could do this.

I hope this is approach you like to go with this package. It is pretty hard to configure all of the different parts.
]

@Jefiozie
Copy link
Contributor Author

Also, I can't figure out why the test fail and the order of files is different there from local.

@Jefiozie
Copy link
Contributor Author

I'm also available to chat about it if you would like that

@Jefiozie
Copy link
Contributor Author

Jefiozie commented Apr 6, 2020

Hi @kamilmysliwiec just checking in to see if this approach is what we want to implement? 😁

@kamilmysliwiec
Copy link
Member

Will review tomorrow! Apologies on the delay

@kamilmysliwiec
Copy link
Member

Could you provide me some feedback and the answer if we need to support nest.json. I don't know if the custom webpack setup is the best way to go as this will break the default behavior of nest start projectTwo. But I don't see a other way we could do this.

I think we can add the custom webpack setup :) Not a big deal

@Jefiozie
Copy link
Contributor Author

@kamilmysliwiec I made the change as requested. What do you think about the extra flag if added --project to determine the project it should be added. Is this a valid approach our should we get it via --sourceRoot

@kamilmysliwiec
Copy link
Member

--project sounds a bit more descriptive (IMO)

@Jefiozie Jefiozie marked this pull request as ready for review April 28, 2020 16:20
@kamilmysliwiec
Copy link
Member

@Jefiozie I can see that you marked this PR as ready for review, but the title states "still wip". 😅 Do you think I can give it a shot already?

@Jefiozie Jefiozie changed the title test: added a lot tests around the monorepo structure still wip test: added a lot tests around the monorepo structure May 23, 2020
@Jefiozie
Copy link
Contributor Author

@kamilmysliwiec oops, I changed it straight away. I think with these changes we have all the scenarios covered. However maybe I'm missing something I tried to make and create as much tests as possible.

@kamilmysliwiec
Copy link
Member

OK thanks! i'll review asap

@kamilmysliwiec kamilmysliwiec merged commit 59cec46 into nestjs:master May 24, 2020
@kamilmysliwiec
Copy link
Member

Looks amazing!

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

Successfully merging this pull request may close these issues.

None yet

2 participants