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

[ADR] Define quality levels #219

Closed
wants to merge 5 commits into from
Closed

Conversation

chibenwa
Copy link
Contributor

@dleangen
Copy link
Contributor

Thank you for this initiative.

What is not clear to me is: is this aspirational, or descriptive?

What I am advocating for is something that is descriptive of the actual state of James, so people new to James know exactly what to expect (or not), both from the code as well as from the community.

@chibenwa
Copy link
Contributor Author

What is not clear to me is: is this aspirational, or descriptive?

It is descriptive.

Note that some features of supported products are however "experimental" or "unsupported", leading to an unclear overview as a user.

An example would be fetchmail usage.

I believe that a better extension packaging would allow to take these experimental/unsupported features out of the default supported products, and would better educate our operators about what is supported or not, and the underlying risks.

What I am advocating for is something that is descriptive of the actual state of James, so people new to James know exactly what to expect (or not), both from the code as well as from the community.

Note that as a consequence, we should open JIRA in order to maintain a clear description of James current status, and encourage contributions on experimental / unsupported components.

This ADR serve as a basis to build this exact diagnostic.

Does it makes sense?

@dleangen
Copy link
Contributor

Thank you. That clarifies a few points, but muddies a few others. Notably, you use the word "supported" in your description, but in the mail discussion there was a fierce reaction to any notion of "support".

From what I understood from the mail thread and if this is indeed descriptive and not aspirational, everything should be labeled as "unsupported", which kind of makes this whole initiative moot. At least I am quite confused.

But then again, producing this ADR is a very good idea because hopefully it will clarify all the confusion. :)

@chibenwa
Copy link
Contributor Author

Notably, you use the word "supported" in your description, but in the mail discussion there was a fierce reaction to any notion of "support".

Good point. Then let's change that terms as it might bring unreasonable expectations from operators.

From what I understood from the mail thread and if this is indeed descriptive and not aspirational, everything should be labeled as "unsupported", which kind of makes this whole initiative moot. At least I am quite confused.

Let's refine our vocabulary to make this cristal clear.


Here are some proposals coming to my mind for a better wording:

  • stable / experimental / unsupported -> I don't like the stable wording as it might give false expectations regarding API stability (which imo is orthogonal to our problem)

  • mature / experimental / unsupported -> maybe better ?

Copy link
Contributor

@Arsnael Arsnael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good initiative, thanks

By the way, in the mail there wasa link to https://github.com/chibenwa/james-project/blob/v3.0_roadmap/v3.0_roadmap.adoc defining what is supported / experimental / unsupported. But it seems quite outdated.

Do we have a more up to date version of this somewhere or is it something that needs to be done too?

src/adr/0040-quality-levels-definitions.md Show resolved Hide resolved
src/adr/0040-quality-levels-definitions.md Outdated Show resolved Hide resolved
@chibenwa
Copy link
Contributor Author

Do we have a more up to date version of this somewhere or is it something that needs to be done too?

We release 3.0.0, so the related roadmap had been carried out.

Copy link
Member

@mbaechler mbaechler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not focus on the "code quality" but on "component quality"
Otherwise, that looks good to me

src/adr/0040-quality-levels-definitions.md Show resolved Hide resolved
src/adr/0040-quality-levels-definitions.md Outdated Show resolved Hide resolved
@chibenwa
Copy link
Contributor Author

I removed the only "code" mention.

src/adr/0040-quality-levels-definitions.md Outdated Show resolved Hide resolved
src/adr/0040-quality-levels-definitions.md Show resolved Hide resolved
Comment on lines +22 to +23
understand the underlying quality of the artifact they use, as well as the level of risk associated,
we need to better define some quality levels.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document defines "quality levels" without defining "quality".

I think that "level" is obvious enough that it doesn't require any further definition, but "quality" may be interpreted differently by different people.

Copy link
Contributor Author

@chibenwa chibenwa Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document defines "quality levels" without defining "quality".

At one point it becomes hard for me to better defines such a generic term. I believes the levels definition disambiguate clearly this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing is really hard, I agree. The hardest part is writing something "objective" that everybody understands the same way, especially in a project like this with people from different countries, backgrounds, cultures, education, generations... We'll never have a completely common understanding, so the trick is to decide the right point at which we should draw a boundary given the time and resources we have available. We will never completely agree on where that boundary should be. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this document, and the "mature" quality level dis-ambiguify what we mean by quality level, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying that "mature" implies "high quality"? In the definitions I proposed, a component could be mature, but unreliable.

I do agree that it's not easy figuring out how all the pieces fit together. Quality really depends on the objective. There is not "absolute". That is why context is so important.


For a given artifact or feature, by **mature** we mean that:

- *interfaces* in components need a contract test suite
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be useful to describe "separation of API/implementation"? It is a term I have used a few times on the list, but I'm not sure I use it in the same way as others. (I have strongly corrupted by OSGi thinking, and it has a very clear meaning over there.)

"Interface" as you use it seems to suggest at the class level, whereas the real "interface" of a component is actualy at the package level, which may be why they call it "API" instead of "interface", to avoid confusion. Usually it is not just a single Java interface that defines behaviour, but rather an entire package. (Should generally not expand beyond a package.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be useful to describe "separation of API/implementation"?

👍 and luckily it is the case almost everywhere.

"Interface" as you use it seems to suggest at the class level, whereas the real "interface" of a component is actualy at the package level, which may be why they call it "API" instead of "interface", to avoid confusion. Usually it is not just a single Java interface that defines behaviour, but rather an entire package

If others don't complain, I will replace "interfaces" terms by "APIs".

- *implementation* of these interfaces need to pass this contract test suite which provides unit tests
- Decent integration tests coverage is needed
- Performance tests need to be conducted out
- Quality Assurance with external clients needs to be conducted out
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "Quality Assurance"? Is that defined in a different ADR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean "Manual Testing". Quality Assurance is in my opinion a common practice of the industry. Defining it is in my opinion not rely needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be surprised as to how something that ought to be clearly defined in the industry creates a lot of conflict. :)

If there is already a clear de facto modus operandi for the James community, or if nobody challenges it, then I guess it's fine. :)

Comment on lines +9 to +21
## Context

We hereby define as an artifact compiled artifact that external people consumes. This includes:

- libraries
- Mail servers
- Extensions for James Mail Servers
- Command line tools

We designate as a feature an optional, opt-in behaviour of a James server that can be configured by
user willing to rely on it.

James as a project delivers several artifacts, and features. In order for project users to better
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem to me like important and fundamental enough concepts to either expand the scope of this ADR, or to have a separate ADR that clearly defines (as a base concept for other ADRs like this one) the concepts of "artifact" and "feature".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the context part does it well enough. If it is not clear enough, please provide alternative wordings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not quite what I meant. I was not commenting on the words, I was commenting on the scope. This ADR is about "quality". However, the terms "artifacts" and "features" seem to be so fundamental that they should not be hidden away in a document that describes something different.

So we should either:

  1. Expand the scope of this document (i.e. it defines "Artifacts", "Features", and "Quality Levels" and not just "Quality Levels", or
  2. Have a separate ADR that defines "Artifacts" and "Features".

Then this can be referred to from other discussions which will surely use these fundamental concepts.

Just something to consider, maybe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a different approach here.

I started another document to explain which server we ship as part of the James project, then I hope that with @ieugen we come up with a list of libraries we deliver. Each one of them will get their own ADR, giving a de-facto definition (and exhaustive list of artifacts).

Regarding features, refining extension mechanisms and planning the work for their extraction would do the same.

Users should have low expectations regarding experimental artifacts or features. They are encouraged to contribute to them
in order to raise its quality.

By **unsupported** we mean that an artifact or feature do not match most of the *mature* quality conditions. Active
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the referenced email, @chibenwa wrote:

This is a fair point, but 'quality' differs from 'support' concerns.

I agree with this point, which is why I have trouble with the "unsupported" label.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propose an alternative please.

I want a term that reflexes the "nothing to expect" concepts. "unsupported" convey this idea quite well to be honest.

Copy link
Contributor

@dleangen dleangen Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote in a separate comment below how I think the concepts are a bit mixed up. But again, there comes a point at which a boundary has to be drawn, so even though IMO it is not entirely clear, I leave the decision up to you. :)

@dleangen
Copy link
Contributor

First, I want to mention again that I think this is a really good initiative. Personally, I think it's really important that people can "know" to some extent what they are getting, especially from a system as huge and important as James.

Here are some proposals coming to my mind for a better wording:

stable / experimental / unsupported -> I don't like the stable wording as it might give false expectations regarding API stability (which imo is orthogonal to our problem)

mature / experimental / unsupported -> maybe better ?

It's difficult nailing down a vocabulary that is commonly shared.

Maybe the orthogonal concepts would be something like:

  • Maturity (mature vs. immature) - The idea that a feature that is either relatively new and in many respects "untested", vs. a feature that has been around for a while, has been seen and used by many eyes, and is considered to be fairly "battle hardened".
  • Formality - Relating to the community requirements regarding whether a feature goes through some kind of formal RFQ process, or if it is just thrown together by a single committer without much oversight.
  • Quality - Relating to whether or not a feature respects the stated design principles of the community (including testing, documentation, code review...).
  • Reliability - Relating to whether or not a feature does what it is stated to do.
  • Support (supported vs. unsupported) - the idea relating to the community's commitment to ensuring that a module / feature (or whatever) remains reliable.

(I just realized that this all implies that the contract be very clearly stated.)

I quickly tested these, and they look at first glance to be orthogonal. (High quality but low reliability vs low quality and high reliability, etc.) They are debatable, so feel free to suggest improvements.

The list "mature", "experimental", "unsupported" seems to mix these concerns a little. But it is difficult to come up with a practical list. "Quality" needs a lot of definition, and likely depends on the other terms.

So based on these, just a thought, but perhaps the first step is to clearly define the "Formality" and "Design Principles" of the community (which to some extent you already do in this ADR), then it would be very easy to map a quality level.

Co-authored-by: Raphaël Ouazana <rouazana@linagora.com>
@chibenwa
Copy link
Contributor Author

What would you think about mature / experimental / immature ?

@ieugen
Copy link
Member

ieugen commented Jun 23, 2020

Kubernetes uses some words to describe the API stability as well https://kubernetes.io/docs/concepts/overview/kubernetes-api/ .
It's nice to llok at what others are doing.
They have a process for moving an API from alfa -> beta -> stable.
But they cover the API (interface) not the implementation.
I believe they consider the implementation stability as part of the API stability.
But they do get a lot more use of their components.

@chibenwa
Copy link
Contributor Author

I would like to make a push and merge this.

Any comments you would like me to address before ?

@ieugen
Copy link
Member

ieugen commented Jul 10, 2020

Let's use it like this and see where it goes.

@chibenwa
Copy link
Contributor Author

Merged

@chibenwa chibenwa closed this Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants