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

Generic Node.js build task #503

Closed
cschweikert opened this issue Apr 8, 2022 · 4 comments · Fixed by #529
Closed

Generic Node.js build task #503

cschweikert opened this issue Apr 8, 2022 · 4 comments · Fixed by #529
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@cschweikert
Copy link
Member

https://github.com/opendevstack/ods-pipeline/blob/master/build/package/Dockerfile.node16-typescript-toolset and https://github.com/opendevstack/ods-pipeline/blob/master/build/package/scripts/build-typescript.sh actually doesn't contain anything TypeScript-specific. The way how it is written is already pretty generic. build-typescript.sh for example awaits certain tasks to exist within the package.json and also certain artifacts to be created. But tasks like build or test would exist in a lot of Node.js-based projects regardless of TypeScript beeing in or out. Also the linting is done with eslint which works for TypeScript as well as plain JavaScript.

I'd propose to treat it generic for any Node.js-based app and rename it to ... builds Node.js-based applications ..., build-nodejs.sh, etc.

In addition I'd propose to provide minimalistic configuration possibilities for the tasks to run. Right now it is npm run build and npm run test, but certain usecases like the e2e-cypress (see also https://github.com/opendevstack/ods-quickstarters/blob/master/e2e-cypress/Jenkinsfile.template) would be fine with just npm run test.

If you like, I'd also be happy to contribute this proposal as an ADR 😉

@cschweikert cschweikert added the enhancement New feature or request label Apr 8, 2022
@michaelsauter
Copy link
Member

I'd propose to treat it generic for any Node.js-based app and rename it to ... builds Node.js-based applications ..., build-nodejs.sh, etc.

@henninggross @netzartist what do you think? I am fine with this.

In addition I'd propose to provide minimalistic configuration possibilities for the tasks to run. Right now it is npm run build and npm run test, but certain usecases like the e2e-cypress (see also https://github.com/opendevstack/ods-quickstarters/blob/master/e2e-cypress/Jenkinsfile.template) would be fine with just npm run test.

Basically what you are proposing is to have a param like test-only that would disable building? We need to find a balance how much customisation we want to offer. For this particular case, I think it might be worthwhile to add. That said, are you aware that the cypress dependencies are not installed anymore in the image used by the task? Therefore, to run cypress, one would need to customise the image anyway, and then it is very easy to adjust the script as well? Do you have other use cases that would benefit from a "test only" behaviour without the need to modify the image?

@cschweikert
Copy link
Member Author

I guess we can even live without customization. Simplest solution would be to provide a dummy build task, e.g.

...
"build": "echo 'no build'",
...

and vice versa for "test", if tests are not set up (yet).

I haven't thought about a proper integration of cypress capabilies yet. But what you said about different customization sounds good to me.

@michaelsauter
Copy link
Member

I like this workaround!

So in my eyes, this issue is then just about renaming the task. I see the following options:

  • ods-build-typescript: current name. communicates an intent, but maybe not most accurate
  • ods-build-nodejs: identifying the base tech, kind of inline with Go/Python
  • ods-build-npm: identifying the build tool, kind of inline with Gradle

I lean towards ods-build-npm, as that implies Node.js, and makes it explicit that you can't bring your own build tool (right now).

For Cypress, I think it would be great to have a FAQ entry how to add this. The place to add it is in those wrapper Dockerfiles, but it would be nice to have a concrete example.

@netzartist
Copy link

Renaming to ods-build-npm is fine for me, too - as well as keeping the npm run build call in the shell-script but allowing for dummy code in the build script in package.json.

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

Successfully merging a pull request may close this issue.

3 participants