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

Numbering sequence can break when different PRs create ADRs at the same time #28

Open
DeniVuMedi opened this issue Oct 12, 2020 · 9 comments

Comments

@DeniVuMedi
Copy link

Let's imagine that the latest number in the ADR numbering sequence is 5.
Dev A creates a PR with an ADR (he/she denotes that ADR with number 6).
Dev B creates a PR with an ADR (he/she denotes that ADR with number 6).
Dev A and Dev B merge at a different point in time.
Now we have two ADRs with different names but the same number.

One possible solution is to have a lock file that would contain the last number. We would have to increment this number each time we create a new ADR. This way after Dev A merges, Dev B can't merge due to conflict that needs to be resolved.
The con of this solution is that a Dev may forget to modify the lock file.

Are there any other propositions on solving this issue?

@koppor
Copy link
Member

koppor commented Oct 14, 2020

For a adr.lock file, tooling needs to be provided. We are currently thinking of a web-based tool to capture and render ADRs. See https://conf.researchr.org/home/icse-2021/score-2021#adr-manager-the-software-architects-favorite-tool for details.

@thomvaill
Copy link

I've been struggling with this conflict issue in my past experiences with markdown ADRs too.
I just released a project that solves this issue, among others: https://github.com/thomvaill/log4brains

Instead of relying on an incremental number prefix (NNNN-title.md), we now prefix the date of creation: YYYYMMDD-title.md. The files are still ordered correctly, while there can't be conflicts during merges anymore.
The tool relies on full filenames to identify uniquely each ADR.

@koppor I am a big fan of your repo. I've been using it a lot to introduce ADRs to the teams I worked with. Such a good reference!
I was planning to get in touch with you just after the release of my project to get your feedback on it. But I didn't know you were planning to develop quite the same tool! I had a quick look at your document, and it seems that Log4brains matches a lot of your specs!! I would be very interested to get in touch with you regarding this. I will send you an email shortly :-)

@koppor
Copy link
Member

koppor commented Dec 11, 2020

Thank you for getting in touch. Looking forward to your mail.

Regarding @adr we could join forces at https://adr.github.io/e-adr/

Regarding the UI: An ADR UI is close to be finished. See https://github.com/koppor/adr-manager for the current development state. Probably one week to go for the first alpha release.

I tried to work on ADR-J for a CLI tool. Work stopped, because of lack of time. Your tool seems to solve that issue.

Finally, using the full filename as ID sounds good. Why not just the dashed title as ID? With your concept YYYYMMDD alone is not a unique identifier and I need to provide all... Do ADRs outdate? 😅

Is it the creation date, the decision date or the merge date? I think the last two make sense, but hard to handle.

Thus, I think, the dashed title should be enough as ID?

@koppor
Copy link
Member

koppor commented Dec 14, 2020

While drafting an ADR, the discussion reminds me on the discussion of BibTeX Keys.

I begin to draft the ADR here:

Options

  • Pattern NNNN-title.md
    • Example 010-use-iso-8601-format-for-date.md and @ADR(10)
    • Good, because consistent to adr-tools
    • Good, because referencing is very easy: @ADR(1). See https://github.com/adr/e-adr.
    • Bad, because conflicts in PR can occur.se #28
  • Pattern YYYYMMMDD-title.md
    • Good, because provides some uniqueness
    • Good, because there is a short id to reference the ADR. E.g., @ADR(20170221)
    • Bad, because unclear how the timestamp should be generated
    • Bad, because merge conflicts can still occur if two ADRs are created at the same time
    • Bad, because no dash separated ISO date
  • Pattern YYYY-MMM-DD-title.md
    • Good, because provides some uniqueness
    • Good, because there is a short id to reference the ADR. E.g., @ADR("2017-02-21")
    • Good, because dash separated ISO date
    • Good, because it follows RFC-3339
    • Bad, because title and timestamp are also separated by -
    • Bad, because unclear how the timestamp should be generated
    • Bad, because merge conflicts can still occur if two ADRs are created at the same time
  • Pattern ShortTitle.md
    • [shorttitle]: The first 3 words of the title, ignoring any function words.
    • Example: @ADR("use-iso-8601")
    • Good, because title is unique
    • Good, because reference to an ADR provides an indication on the content
    • Good, because usage in @ADR annotation is "OKish", because only the first three significant words are used
  • Pattern .md
  • Pattern title.md

Side notes:

  • Make it configurable
  • Convention per project. Decide for one style and keep.
  • What about sub systems / categories? --> Add categories #7
  • URI-based? category/title.md --> docs/adr/category/title. This is (more or less) ADR-0010 / issue Add categories #7
  • ADR-0010, ADR-support-categories, ADR-SC, ADR-SupportCategories.
  • ADR-categories-by-sub-folders.
  • Names are immutable
  • Discussion: If a change in the decision occurs (undeside, redecide), a new ADR with a new ID has to be created. The "old" ADR has to point to a new one.
  • What

Notes

@thomvaill
Copy link

Sorry for the late reply! I just sent you the email ;)

Originally, I went with the title.md option, but it was really hard to find the file you were looking for in the file explorer of your IDE because they are not ordered at all!
Prefixing them with a format like YYYYMMDD (YYYY-MM-DD would also work) makes them nicely sorted.
That's why by default I use this prefix, with the file creation time, because even if this date is not the most relevant one, it's only used for rough sorting, and I wanted to avoid having the rename the files.
But it's not required in Log4brains to use this format because it uses the full filename as a "slug", to identify an ADR. All you need is that it must be unique, but it's already enforced by the filesystem.

But you are right, the downside is that you need to type the full title to reference an ADR, which is painful and error-prone.
I like your ShortTitle.md option, because it makes the slugs shorter, but still meaningful. Especially for short titles like "Use Lunr for search", "Use xxx for yyy". However, it's more tricky when dealing with long titles that describe specific architecture cases, like:

  • Before: 20201026-the-core-api-is-responsible-for-enhancing-the-adr-markdown-body-with-mdx.md
  • After: 20201026-core-api-responsible.md

In this case, it's not really meaningful.

I will try some experiments with the Log4brains CLI. Maybe the solution is to propose a simplified slug without function words that the user can edit at creation.

@ms-ati
Copy link
Contributor

ms-ati commented Dec 22, 2020

Are there any lessons that can be learned from the prior experience of Ruby on Rails Migrations?

The tooling is very similar, and the solution of YYYYMMDD prefixing of the short names was adopted for similar reasons.

@ms-ati
Copy link
Contributor

ms-ati commented Jan 2, 2021

Another alternative is the experience of Rust RFCs:

Copy 0000-template.md to text/0000-my-feature.md (where "my-feature" is descriptive). Don't assign an RFC number yet; This is going to be the PR number and we'll rename the file accordingly if the RFC is accepted.

@koppor
Copy link
Member

koppor commented Jan 4, 2021

@ms-ati I seems the Rust RFCs assume a single repository (the RFC repository). What is the single ADR repository? How does that relate to the discussion at #10, where a sub directory for each ADR category is assumed? Maybe, it's straight forward and the numbering is per category. Then, the only thing left are code-references with @ADR(0000). Should the merge also change all code occurrences at the merge? What if two ADRs are written in a PR? Maybe, they should be TMP1, TMP2, ... then? Sounds not too bad.

@koppor
Copy link
Member

koppor commented Mar 2, 2021

A new pro for numbering: When creating 100 decisions, they can be referred to easily when discussing. It is more easy to say "ADR-100" than "ADR-gabt" and "ADR-2020-12-10". Moreover, when using numbers, ADRs can be grouped by number blocks. Say 1 to 19 discusses the URL structure, 50 to 100 the mapping of internal data sources to the URLs, ...

Thus, it seems a matter of taste and tooling on ADRs should support different styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants