Requirements for Contributing.md file

[coot]

In a recent discussion @kevinhammond and me came up with the following list of requirements for the CONTRIBUTING.md:

• major contributions must provide a detailed design that can be reviewed by domain experts, and a reporting showing how the review recommendations have been addressed (where feature changes are being made, it is recommended that the process should start with a community CIP before commencing detailed design work);
• major contributions must have undergone an outline security audit prior to implementation, and a report provided on how the recommendations have been met;
• all contributed code must meet the coding standards that are given in the relevant code repositories;
• all contributions must pass all the standard tests, including QuickCheck, integration and end-to-end tests;
• all contributions must pass the standard performance tests;
• any new features must be supported by new tests that are contributed with the change (Quickcheck or end-to-end tests as appropriate);
• the code must have been accepted by the code maintainer;
• a specific security code review must be carried out for any major contributions;
• the contribution must adopt the same open source licences as the repositories where it is contributed.
• the contributor is responsible for updating documentation and design documents and it can expect guidance from the maintainer

I think it would be nice to add a section or a standard CONTRIBUTING.md document in Cardano Engineering Handbook, which could be used as a basis by other projects. Currently, very few projects have that document.

[coot]

Consensus has interesting Haskell/GHC tips & tricks in its contributing document. I think they should be put upstream, maybe as a section in Cardano Engineering Handbook, as they are general enough not just for consensus contributors. @jasagredo & @dnadales what do you think? You could then include just a link in your contributing guide to it (as other projects could do too).

[dnadales]

That sounds sensible.

[coot]

Some examples of CONTRIBUTING.md files which we have:

• Cardano-Node Contributing Document
• Ouroboros-Network Contributing Document
• Cardano-Ledger Contributing Document
• Plutus Contributing Document

[dorin100]

I also think that all the above steps are important but I don't think we have, right now, a single space explaining (or giving visibility on):
what are all the required tests per Product - Cardano in this case (per component, property, integration, system, e2e, etc),
how all these tests can be run, how to interpret the results, etc

One option could be to use the page my team created to give visibility on all the testing that we do on Cardano (+ a small summary on how to run them) - that way we would have all the testing details in the same place.

This approach would also help internal stakeholders. but also the community, to have a better understanding of what is done/tested/automated.

Once we open the testing tools, we can have different types of contributions (I suggest having a guide also for such proposals):

• new functionalities (PRs, etc)
  or
• proposals to improve the test coverage at different levels
  or
• tests improving the test coverage
  or
• new test tools/generators

[michaelpj]

I think this would be a great candidate for a section in "Practices". The idea there is that we can give examples of things that we do for inspiration. I think it's very hard to point to any of the things on your list and say that they should be policy (except maybe "PRs should pass the tests", which is usually enforced by CI anyway). At the moments our projects take quite different approaches, and I think trying to standardise them or give a default example is tough.

Perhaps we could write something like this:

Many of our projects have extensive CONTRIBUTING documentation. For example:

Here is a list of topics that you may want to cover in your CONTRIBUTING documentation:

• Under what circumstances does a proposed change require a design discussion beforehand? Is a ticket sufficient? Does the design need to be signed off by someone before an implementation will be accepted?
• Under what circumstances does a change require a security audit? Whatever is written here should be compatible with the Audit policy (link TBD), for most projects it should be sufficient to link to that.
• Are there any additional non-automated tests that need to be run for certain kinds of change, e.g. performance tests?
• etc.

[coot]

@dorin100 You are right that we don't have such document which says which tests must be written per component. I don't know how the situation looks like in different teams, but the general policy we have is that we only release components if we have sufficient tests and that's decided by the developers. In this regard it's implicit that all the test are the required ones, there are some exceptions to this rule (e.g. some test are flaky etc...).

I am not sure if general audience / community is that interested in the tests we have. I think a much more interesting would be to write a publishable technical report on some of our testing techniques, what kind of bugs we caught early.

@michaelpj as always you put things in the right words :). What about posting somewhere a template which can be copy pasted / edited to ones need. Maybe on a wiki and just include a link which we cite in the handbook.

[dorin100]

@dorin100 You are right that we don't have such document which says which tests must be written per component. I don't know how the situation looks like in different teams, but the general policy we have is that we only release components if we have sufficient tests and that's decided by the developers. In this regard it's implicit that all the test are the required ones, there are some exceptions to this rule (e.g. some test are flaky etc...).

I am not sure if general audience / community is that interested in the tests we have. I think a much more interesting would be to write a publishable technical report on some of our testing techniques, what kind of bugs we caught early.

I think the opposite :), for example,

• if I would be a (dApp) developer I would be interested to run all IOGs tests as a regression for the new functionalities I would write + adding my own ones
• if I would be SPO, I would be interested in all the tests that IOG is running (to review/contribute (to) them, to propose new ones, to find gaps (if any), etc)
• if I would be an Exchange, Wallet I would be interested to see that IOG already runs all the scenarios my business is based on and even more propose new scenarios to avoid future regressions that could affect my business

The feedback I got from the community people in Lisbon was exactly this - they don't have (but want) visibility on the tests and the test tools owned by IOG. They were proposing to be more involved in the testing process because they were directly interested in the quality and stability of the project (they have businesses now that are depending on this).

• when I am speaking about providing visibility on tests, I am referring to something as simple as having a short doc in each repo (that would be aggregated in 1 common place) saying where the tests for that component lives and when and how they are run - we already have lots of automated tests per component but at this moment we only have this info as common knowledge inside each of the teams.

[michaelpj]

I am referring to something as simple as having a short doc in each repo ... saying where the tests for that component lives and when and how they are run

Don't we have a fully-generic answer to this for every Haskell project? The tests live in test suites, described in the cabal files, and are run with cabal test <component>. And furthermore, this is totally standard - anyone who has ever written any Haskell will know this. So I'm not sure what else there is to say?

[dorin100]

Don't we have a fully-generic answer to this for every Haskell project? The tests live in test suites, described in the cabal files, and are run with cabal test . And furthermore, this is totally standard - anyone who has ever written any Haskell will know this. So I'm not sure what else there is to say?

I am not sure, but considering that there are a lot of teams and not all people involved in Cardano are Haskell developers, I think it would increase the trust (with the community, management, stakeholders, etc) to remove the assumptions and make this small info clear in a written form.

[coot]

@dorin100 I think these questions should be answered in CONTRIBUTING.md file, in particular cardano-node should mention cardano-node-test repo and the benchmarking suite (although that's run by us, and since it requires access to enough resources might to be easy for some contributors to run on its own).

There are various audiences here: Haskell developers & devops (where SPO, or exchanges fall into). First one would know how to find test a test suite and how to run it; the latter, I suspect the latter one is more interested in cardano-node-test rather than low level testing suites. cardano-node-test will be open-source (if isn't already), that will improve the situation for them.

If SPOs or exchanges would like to do an audit of component level test suits, they need to have Haskell experts, which is rather unlikely. Although we should try to lower the bar.

This actually brings another point to our coding standards: our test suits ought to be well documented, more of a first class citizen, instead like it happens in many projects a bit neglected and hacky part of the code base.

[michaelpj]

This actually brings another point to our coding standards: our test suits ought to be well documented, more of a first class citizen, instead like it happens in many projects a bit neglected and hacky part of the code base.

Getting a bit nebulous here... but I'm not sure something this broad is something you can make happen by writing it down. Either it's part of your team culture or it isn't. Writing it in your coding standard won't make it happen if isn't happening. (As opposed to something like "all let-bindings should have type signatures" or something, which is clearly enforceable)

[michaelpj]

I'm going to draft a PR with a CONTRIBUTING practices section.

[michaelpj]

#34