Skip to content

Conversation

bdovaz
Copy link
Contributor

@bdovaz bdovaz commented Jul 5, 2025

Summary

Encapsulate the implementation detail of how the revision is resolved through a node.

TODO

  • ...

Checklist

@bdovaz bdovaz requested a review from a team as a code owner July 5, 2025 20:35
@bdovaz bdovaz requested review from ArtificialOwl, artonge and sorbaugh and removed request for a team July 5, 2025 20:35
@bdovaz bdovaz changed the title Get revision chore: get revision for a node Jul 5, 2025
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Not sure how useful this is, but it certainly won't hurt either.

@provokateurin provokateurin added enhancement 3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update labels Jul 5, 2025
@provokateurin provokateurin added this to the Nextcloud 32 milestone Jul 5, 2025
@provokateurin provokateurin requested a review from come-nc July 5, 2025 20:56
@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 5, 2025

Not sure how useful this is, but it certainly won't hurt either.

As I put in my original issue, the utility is not having to know the detail of how the revision int is resolved.... The APi of file_revisions is not intuitive in this sense...

I had to go to VersionAuthorListener to deduce how a revision is resolved from a node (getMTime)

Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Please squash your commits and format the commit message to use conventional commits.

@bdovaz bdovaz changed the title chore: get revision for a node chore(files_versions): get revision for a node Jul 7, 2025
@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 7, 2025

Please squash your commits and format the commit message to use conventional commits.

I understand that you are referring to the DCO issue. Resolved!

@susnux
Copy link
Contributor

susnux commented Jul 7, 2025

I understand that you are referring to the DCO issue. Resolved!

Yes but please also squash them into one commit and give them a proper commit message according to conventional commits.

@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 7, 2025

Yes but please also squash them into one commit

Are there any instructions on which git commands to use in a readme file in the repository? I've never done this before manually.

Whenever I squash, it's when merging the PR through GitHub or Azure DevOps tools.

@susnux
Copy link
Contributor

susnux commented Jul 7, 2025

Are there any instructions on which git commands to use in a readme file in the repository? I've never done this before manually.

Squash:

# update references
git fetch --all
# interactive rebase - adjust `origin` with the name of the upstream repository (this one here)
git rebase -i origin/master
# now in the opened editor mark all except the first as "s" (for squash)
# save and follow the instructions git prompts you with
git push -f

@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 7, 2025

@susnux thank you, I think I did it right.

@provokateurin provokateurin enabled auto-merge July 7, 2025 11:32
@provokateurin provokateurin self-assigned this Jul 7, 2025
@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 7, 2025

@provokateurin I must have done something wrong. Do you know what it could be?

@provokateurin
Copy link
Member

The class LegacyVersionsBackend also needs to have the getRevision method implemented to fulfill the interface.

auto-merge was automatically disabled July 7, 2025 12:08

Head branch was pushed to by a user without write access

@provokateurin
Copy link
Member

@bdovaz please squash again, we can't merge it like this.

Signed-off-by: Borja Domínguez Vázquez <[email protected]>
Signed-off-by: Borja Domínguez Vázquez <[email protected]>

wip

Signed-off-by: Borja Domínguez Vázquez <[email protected]>
Signed-off-by: Borja Domínguez Vázquez <[email protected]>

wip

Signed-off-by: Borja Domínguez Vázquez <[email protected]>
Signed-off-by: Borja Domínguez Vázquez <[email protected]>

Update IVersionBackend.php

Signed-off-by: Borja Domínguez Vázquez <[email protected]>

wip
@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 7, 2025

Ready (I hope)

@provokateurin provokateurin enabled auto-merge July 7, 2025 12:34
@bdovaz
Copy link
Contributor Author

bdovaz commented Jul 7, 2025

Are the problems that arise caused by my PR? Because apparently it doesn't seem so.

@provokateurin
Copy link
Member

No, they can be ignored because your PR is coming from a fork.

@susnux susnux changed the title chore(files_versions): get revision for a node feat(files_versions): get revision for a node Jul 7, 2025
@sorbaugh sorbaugh disabled auto-merge July 7, 2025 15:29
@sorbaugh sorbaugh merged commit a4879f2 into nextcloud:master Jul 7, 2025
196 of 202 checks passed
@bdovaz bdovaz deleted the get-revision branch July 7, 2025 15:29
max-nextcloud added a commit to nextcloud/collectives that referenced this pull request Jul 8, 2025
mejo- pushed a commit to nextcloud/collectives that referenced this pull request Jul 8, 2025
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement feedback-requested pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a more secure method of obtaining the revision id from a Node when file versioning is used.
5 participants