commit client/dist folder to repo?
chrisdias opened this issue · 13 comments
is there a reason that the /client/dist folder is not checked in? i know its build with gulp but always end up with a bunch of new files in Git or we need to add the folder to .gitignore.
what is the right approach?
@chrisdias I'd vote to not check-in the build output, as we have it currently. #31 updates the .gitignore file to exclude the /client/dist folder, and it make be worth just making that change now, instead of coupling it to the auth service.
Did you want to check in the /client/dist folder? For your Build demo, are you deploying the Sticker App as a custom container, or are you doing git-based deployment to Kudu?
I was going to deploy the app both ways - start with git and then move to a container, but we still need to figure out if VSTS will pick up the existence of a gulpfile.js. If not, then we have to either tweak the build definition (not ideal, have to go to portal) or check in this folder, which is a workaround until VSTS can figure this out automatically.
@chrisdias I just updated the .gitignore to work around the current issue. The CLI changes indicate that you can specify the app uses Gulp to build (see below), so I'm hoping this should work without you needing to go to the portal.
@lostintangent playing with this more today... when you deploy from local git directly to the website, the gulp command is not run, so the site doesn't work. My plan was to do a simple deploy from Git directly, then move to vsts.
what do you suggest we do in this case?
@chrisdias In order to reflect what users would actually need to do to support this scenario, we should create a custom Kudu deployment script that runs "npm run build" after doing the "npm install". We can use the following tool to create the base deployment script and modify it as mentioned: https://www.npmjs.com/package/kuduscript.
I can make this change tonight, at which point, you can git deploy the app, and it will build it in App service.
@lostintangent this is why containers rock, so much more straightforward to do all this. what do you do in azjs? do you look for a gulpfile and just assume the default task?
the reality of it all is that the git deployment directly is not a good way to go, regardless of whether or not you need to run gulp. i shouldn't even demo it, except maybe to make a point as what not to do. i feel like adding a .deployment file pollutes the repo. thoughts?
I completely agree. The Git deployment method, that does a build, isn't something I would ever really recommend :) I just want to make sure you have what you need for your demo! If you're cool with it, I'd vote against the local Git solution and make the point that it works for simple cases, but breaks down quickly, which is why having a "true" CI/CD solution and a BYOC solution are so important.
Yes, Azjs does the custom deployment script magic for you, since it's not a great experience to need to manage them yourself :)
maybe i demo the "30 sec 0 to hero" flow and then say "don't do that ever again!" :)
Just let me know if you need the custom deployment script, and I'm happy to help create it :)
BTW, as a total aside: I'm also going to be contributing the StickerApp as a template for the new Linux Try app service experience, that will allow devs to spin up a Linux web app, pre-provisioned with this app, and they'll have access to the CLI and portal for 30 minutes.
@chrisdias Actually, could we set a "postinstall" script in the package.json file that runs "npm run build"? That way the repo doesn't get cluttered with Kudu-specific deployment artifacts, but would allow you to do the Git deployment solution in your demo.
i looked into this a bit, but i concluded it wouldn't work because the default kudo deployment runs npm install --production, so gulp and whatever else in devDependencies isn't going to be there. could move that to dependencies but it is really then becoming a demo and not reality.
Could use this: https://www.npmjs.com/package/postinstall-build
But like you said, it starts to get a little complicated :)
Closing this as resolved per our conversation.