OpenElements / BenchScape

Collect and analyze JMH benchmark results for continuous integration
https://app.benchscape.cloud
Apache License 2.0
3 stars 3 forks source link

Add development/contributor documentation #391

Open ascheman opened 5 months ago

ascheman commented 5 months ago

Currently, this is work-in-progress and needs some discussion by the team!

First review by @hendrikebbers required.

I recommend opening the new CONTRIBUTING.adoc in IntelliJ (with AsciiDoc and PlantUML installed), or to start with the current version as PDF.

hendrikebbers commented 5 months ago

Adding @Ndacyayisenga-droid and @Jexsie for discussion.

Based on my current knowledge I would favor the following behavior:

ascheman commented 5 months ago
  • No git hooks are added by default. It is fine to have a script for that and make it possible to add the hooks locally but it should not be added "by default".

As we seem to have different types of development persons (frontend vs. backend, NPM vs. Maven) we either should find something which make everyone happy or make it optional (as proposed in the guide).

  • The NPM build should be startable independent of the Maven build (calling npm directly from frontend folder)

👍

As Maven installs the respective (POM.xml configured NPM and Node binaries) to a local path (frontend/node/node and frontend/node_modules/npm/bin/npm), we could use those versions locally as well by adding the respective directories to ${PATH}. This would ensure that the very same version is used even locally (as long as Maven was called prematurely).

But I am no frontend developer, we should handle it in a way which is most convenient to the frontend people.

  • A Maven call (mvnw verify) should call the npm build, too. Here it would be perfect to have a profile that ignores all npm steps (@ascheman a good issue for future)

This is already the case, and we could easily add a respective profile. Feel free to file an issue for that and assign it to me for future work.

  • I like the setup-benchscape.sh and start-benchscape.sh scripts in the root folder. that files should stay.

I would at least recommend to unify and simplify them. As stated in the guide, setup-benchscape.sh already builds AND starts the services. Therefore, start-benchscape.sh seems redundant (as I would think, for local development you only want to start both applications with a fresh build)?

But again, frontend developers should say what they use in their daily work and which other use cases they perhaps have?

  • I do not see a benefit the Makefile in addition. We should get rid of that (@Jexsie do I miss something here)

💯

ascheman commented 5 months ago

If you don't mind, I would even unify the README (.md) and the new CONTRIBUTING guide (and move all developer centric docs to the latter).

Jexsie commented 5 months ago

Thanks, @ascheman and @hendrikebbers, we totally agree with the above suggestions. The main reason for the Makefile was to have something that can be easily read and remembered, we can ignore it still.

As Maven installs the respective (POM.xml configured NPM and Node binaries) to a local path (frontend/node/node and frontend/node_modules/npm/bin/npm), we could use those versions locally as well by adding the respective directories to ${PATH}. This would ensure that the very same version is used even locally (as long as Maven was called prematurely). But I am no frontend developer, we should handle it in a way which is most convenient to the frontend people.

This sounds technically fine. I won't say I got what you meant 🙂, but yeah, sounds good to me.