Skip to content

[review] Expand revision count limit from 1000 to 10000 entries#69

Merged
ml-ebs-ext merged 3 commits into
testfrom
342810-expand_review_revision_limit
May 8, 2026
Merged

[review] Expand revision count limit from 1000 to 10000 entries#69
ml-ebs-ext merged 3 commits into
testfrom
342810-expand_review_revision_limit

Conversation

@ml-ebs-ext
Copy link
Copy Markdown
Contributor

@ml-ebs-ext ml-ebs-ext commented Apr 30, 2026

The simplest solution is to bump the number of digits dedicated to revisions from three to four. That would allow studies to have ten thousand data revisions, which would mean one revision per day for more than 27 years.

The only issue is what happens to studies that are already in progress. Instead of forcing them to migrate from the old naming scheme to the new one, what we can do is:

Every new .base file that we create gets four digits (e.g. domain_0000.base). Only newly added domains create .base files. Old ones are OK having only three digits.

Every new .delta file gets the amount of digits that its .base counterpart has. This means that old studies keep using three-digit .delta names and new ones get four digits.

I've tested this approach against the “review oracle”, letting it create 1500 revisions and it works. I won't leave that change in the codebase, because it takes ages to run:
image

Critical checks

  • Is the test version number correct (x.x.x-9000)?

  • DESCRIPTION file

  • NEWS.md

  • Does the build pass?


Documentation

Does it include the following sections?

  • Module introduction with features

    • (O) Screenshots
  • Installation details

  • Explanation of function arguments

  • Data specifications and requirements

  • Different possible visualizations

  • Are the changes/new features included in NEWS.md?

    • (O) Screenshots
  • (O) Explanation of input menus

  • (O) Short articles on building the app, compatibility with other modules, known bugs,...


QC Report

  • Does it include a QC Report with positive outcome?

  • Are the new features reflected accordingly in the specs?


API conventions

  • Follows API convention

@ml-ebs-ext ml-ebs-ext requested a review from yashshah1995 April 30, 2026 11:12
@ml-ebs-ext ml-ebs-ext requested a review from a team as a code owner April 30, 2026 11:12
@yashshah1995
Copy link
Copy Markdown

@ml-ebs-ext
I got different results from what you have mentioned in comments. I only added rs later but all other old domains also got additional base files.

image

Shall we take a look at it?

@ml-ebs-ext
Copy link
Copy Markdown
Contributor Author

ml-ebs-ext commented May 7, 2026

@ml-ebs-ext I got different results from what you have mentioned in comments. I only added rs later but all other old domains also got additional base files.

@yashshah1995 Thanks for catching that. Not one of my brightest commits. I've pushed a fix. Let's see if my brain works today!

Copy link
Copy Markdown

@yashshah1995 yashshah1995 left a comment

Choose a reason for hiding this comment

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

💯 Checked

@ml-ebs-ext ml-ebs-ext merged commit 8679add into test May 8, 2026
5 checks passed
@ml-ebs-ext ml-ebs-ext deleted the 342810-expand_review_revision_limit branch May 8, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants