Skip to content

feat(docs): file modification#888

Open
carlinmack wants to merge 4 commits intoinveniosoftware:masterfrom
carlinmack:file-modification
Open

feat(docs): file modification#888
carlinmack wants to merge 4 commits intoinveniosoftware:masterfrom
carlinmack:file-modification

Conversation

@carlinmack
Copy link
Copy Markdown
Contributor

@carlinmack carlinmack commented Nov 13, 2025

❤️ Thank you for your contribution!

Description

Please describe briefly your pull request.

Release notes

image

Customize file modification (might look less squashed if you open image in new tab)

image

Checklist

Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:

  • I'm aware of the code of conduct.
  • I've created logical separate commits and followed the commit message format.
  • I've targeted the master branch.
  • If this documentation change impacts the current release of InvenioRDM, I will backport it to the production branch following approval or indicate to a maintainer that it should be backported.

Reminder

By using GitHub, you have already agreed to the GitHub’s Terms of Service including that:

  1. You license your contribution under the same terms as the current repository’s license.
  2. You agree that you have the right to license your contribution under the current repository’s license.

@carlinmack carlinmack moved this to In progress in Sprint Q2 2026 Nov 13, 2025
@carlinmack carlinmack moved this from In progress to In review 🔍 in Sprint Q2 2026 Nov 14, 2025
@carlinmack carlinmack marked this pull request as ready for review November 14, 2025 10:09
@carlinmack carlinmack changed the title WIP: feat: file modification feat: file modification Nov 14, 2025
@carlinmack carlinmack changed the title feat: file modification feat(docs): file modification Nov 14, 2025
Second, the message which is returned to the user when they have run out of time to publish is defined via your config:

```
RDM_FILE_MODIFICATION_VALIDATION_ERROR_MESSAGE = _(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This configuration variable should be avoided, it is bringing more complexity to the config - it opens the door for creating thousands of variables of the same category, would we normally create a config variable for every error message we have in the system?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is true, and the configuration variable is not necessary since you can always override what should be translated. this works for the English language too.

so this config variable is not necessary this should be replaced with the value of the variable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks for the feedback, made this PR inveniosoftware/invenio-rdm-records#2257

will update the docs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the docs, ready for review/merge

@carlinmack carlinmack moved this from In review 🔍 to In progress in Sprint Q2 2026 Feb 16, 2026
@carlinmack carlinmack self-assigned this Feb 16, 2026
@carlinmack carlinmack moved this from In progress to In review 🔍 in Sprint Q2 2026 Feb 16, 2026
Copy link
Copy Markdown
Member

@palkerecsenyi palkerecsenyi left a comment

Choose a reason for hiding this comment

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

Looks very nice and clear now, thanks!!

@palkerecsenyi palkerecsenyi removed their assignment Feb 17, 2026
Copy link
Copy Markdown

@juliehinge juliehinge left a comment

Choose a reason for hiding this comment

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

Looks good and clear :)

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

Labels

None yet

Projects

Status: In review 🔍

Development

Successfully merging this pull request may close these issues.

5 participants