OPTIMADE icon indicating copy to clipboard operation
OPTIMADE copied to clipboard

Preview version of the OPTIMADE standard

Open JPBergsma opened this issue 2 years ago • 3 comments

There are currently many PR’s on the OPTIMADE GitHub that are more or less finished, but have not yet been merged with the develop or the master branch. A number of examples of this are:

SMILES property #392 Adding the trajectories endpoint to OPTIMADE #377 Property definitions #376 Files endpoint #360 Allow boolean values in queries #348

For database providers who want to add some kind of functionality to their database that is not yet in the current master branch, it would be good to know what kind of ideas there are and how these will most likely be implemented in future versions of the OPTIMADE standard, so they do not need to reimplement this solution when the method in the standard differs from the method they chose.

Giovanni therefore thought that it would be a good idea to have a “preview” version of the OPTIMADE standard with the changes that are likely to end up in the final standard, even though they may still undergo some small changes. The approval requirements for such an addition would be lower than those of the master branch, but the PR should have been discussed, and written in an almost final form, ideally already with a proof of concept implementation.

In principle I think this could be a good idea. There are however still a few things that would need to be discussed.

  1. How many people should approve a change before it gets merged ?

For the main branch we now request that two persons accept the change. Perhaps one would be enough for this preview version.

  1. Should all changes be included in this preview or only those that are backwards compatible?

In case there are changes that are not backwards compatible, the major version number, under which the OPTIMADE API with this particular change is served, should be increased. This can only be done once we know in which major release the change will be. In practice all the changes we have made so far are backwards compatible, and major releases will probably be separated by considerable amounts of time, so this may not be a big problem in practice.

  1. How should we implement this in the current GitHub environment?

If we were to merge the Pull request it would automatically get closed and a new PR should be made to continue discussing / editing the code, which would fragment the discussion. This may not be a disadvantage though, some of the discussions become very long, so starting a new topic with a summary of what has already been discussed would make it easier for new people to join the discussions.

Another solution may be to have a separate repository in which separate pull request can be made to add the changes to the preview version.

  1. How are we going to version these preview additions?

It seems reasonable to allow that the changes are added to the main branch in a different order than they were added to the preview branch. The preview version of the OPTIMADE standard will probably be branched of the master branch so perhaps we could use the version number of the masterbranch on which it is based as a start and subsequently add an extra version number to indicate The current OPTIMADE version is 1.1.0. we could then label the first preview version as 1.1.1-p.1 Here the patch version number has been updated to indicate that this release is newer than the 1.1.0 release. if a second preview version would be made we could label this as 1.1.1-p.2 etc. If a minor update is made to the OPTIMADE specification is released it's version would be come 1.2.0 the version number for the accompanying preview version would than become 1.2.1-p.2 indicating that it is still the same preview version but now based from a newer master version.

We could discuss this further at the OPTIMADE meeting at the end of May/ beginning of June. I am curious to hear what your opinion about this is and how you would implement it.

JPBergsma avatar May 17 '22 15:05 JPBergsma

This is an interesting proposal, which I believe could benefit the community of developers and adopters. Surely there are concerns to respond before we see this happen.

  1. How many people should approve a change before it gets merged ?

Do you mean approve as in approving an introducing PR? Well, then even one is quite much. The PRs you have listed rarely have even a single full approval. Can we somehow identify PRs nearing consensus and preview them?

  1. Should all changes be included in this preview or only those that are backwards compatible?

I think features breaking the backwards compatibility are less likely to be approved and integrated soon. Thus these should display higher level of approval before being included into feature preview.

Another question concerns conflicting features. There may be features that contradict one another, or affect the same portion of the specification.

  1. How should we implement this in the current GitHub environment?

We could have a preview branch and merge in not the PRs, but the branches associated with them. This surely would increase the manual work, but I do not see a solution which would not.

  1. How are we going to version these preview additions?

Why not append an ever-growing list of PR numbers? I.e., 1.1.1-pre.392.377.376.360.348 lists all the PRs from the list above.

merkys avatar May 30 '22 09:05 merkys

Do you mean approve as in approving an introducing PR? Well, then even one is quite much. The PRs you have listed rarely have even a single full approval. Can we somehow identify PRs nearing consensus and preview them?

I indeed meant the number of persons required to approve a PR. I was wondering whether we could perhaps do something along the lines of. "If no one disagrees, I will add this to the preview version in two weeks from now." That should give people enough time raise any objections. I am not aware of a way to enforce this in Github though.

Good point about the conflicting PR's, Most PR's are orthogonal but there may indeed be cases where there is a conflict. Ideally people in oldest PR would notify those that wrote the newer PR that there is a conflict. So it would not get merged without solving the conflict. But there is off course no guarantee this case would be handled correctly.

We could have a preview branch and merge in not the PRs, but the branches associated with them. This surely would increase the manual work, but I do not see a solution which would not.

Yes I think that could work.

Why not append an ever-growing list of PR numbers? I.e., 1.1.1-pre.392.377.376.360.348 lists all the PRs from the list above.

I think that could work, if no PRs are removed from preview version, without merging them in the main branch. Otherwise, it would become hard to see which preview version is newer.

JPBergsma avatar Jun 01 '22 17:06 JPBergsma

I indeed meant the number of persons required to approve a PR. I was wondering whether we could perhaps do something along the lines of. "If no one disagrees, I will add this to the preview version in two weeks from now." That should give people enough time raise any objections. I am not aware of a way to enforce this in Github though.

We should take into account the added complexity of this suggestion with regard to added benefit. I am pretty sure I could get around without the preview version by simply stating, for example, "this is based on OPTIMADE v1.1.0 with #360 and #376 included".

Good point about the conflicting PR's, Most PR's are orthogonal but there may indeed be cases where there is a conflict. Ideally people in oldest PR would notify those that wrote the newer PR that there is a conflict. So it would not get merged without solving the conflict. But there is off course no guarantee this case would be handled correctly.

I would say the onus should be on the developers of the newer PR to discover and cure the conflicts.

Why not append an ever-growing list of PR numbers? I.e., 1.1.1-pre.392.377.376.360.348 lists all the PRs from the list above.

I think that could work, if no PRs are removed from preview version, without merging them in the main branch. Otherwise, it would become hard to see which preview version is newer.

Removing a PR from preview version sounds tricky. This should rarely be done.

merkys avatar Jun 02 '22 07:06 merkys

I think lots of good points were raised here but somehow (nearly 2 years later) we've muddled through a lot of the big changes, so I will close this issue for now.

ml-evs avatar Mar 25 '24 13:03 ml-evs