Skip to content

document persistent major version feature#840

Merged
mmattel merged 9 commits intomasterfrom
feature/persistent-major-versions-v2
Feb 13, 2023
Merged

document persistent major version feature#840
mmattel merged 9 commits intomasterfrom
feature/persistent-major-versions-v2

Conversation

@mrow4a
Copy link
Copy Markdown
Contributor

@mrow4a mrow4a commented Feb 10, 2023

References:
owncloud/core#40531 (Implement persistent major version workflow (v2))
owncloud/core#40636 ([docs-only] Fix headline in config.sample)

Document the persistent major version feature.

Needs a config-to-docs run (see commit config-to-docs run).

No Backport

@mmattel
Copy link
Copy Markdown
Contributor

mmattel commented Feb 10, 2023

@mrow4a the change in config_sample_php_parameters.adoc needs to be removed and instead added to core. You can do that in the open PR in core. I will then do a config-to-docs run which transports the changes to docs.

Copy link
Copy Markdown
Member

@jvillafanez jvillafanez left a comment

Choose a reason for hiding this comment

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

I'd change the image. The first time I saw it I thought the "publish version" below the check mark was a button, not a tooltip, so I was surprised when I didn't see the button.
I don't know how we can make it clear that it's a tooltip, so I think it's better to remove from the image. Just take the same screenshot without the tooltip showing, and maybe use a bit more realistic usernames

@mrow4a
Copy link
Copy Markdown
Contributor Author

mrow4a commented Feb 10, 2023

@mrow4a the change in config_sample_php_parameters.adoc needs to be removed and instead added to core. You can do that in the open PR in core. I will then do a config-to-docs run which transports the changes to docs.

@mmattel I am afraid I do not understand.

@mrow4a
Copy link
Copy Markdown
Contributor Author

mrow4a commented Feb 10, 2023

@mmattel what you mean is https://github.com/owncloud/core/blob/81e3c8b7e741d708f984b6c500eaadbe1dce348a/config/config.sample.php#L717-L723 ? It is already done? So it will be automatically done in config_sample_php_parameters.adoc ?

@mmattel
Copy link
Copy Markdown
Contributor

mmattel commented Feb 10, 2023

just not in front of my nb, but any changes related to config sample must be made in core. if they already exist, fine. if in this pr, fine. if not, must be added.
any changes manually added in docs, will be automatically removed on the next config-to-docs run.
a config-to-docs run will then transport any changes made to docs which I will do. for details see the config to docs repo.

@mrow4a
Copy link
Copy Markdown
Contributor Author

mrow4a commented Feb 10, 2023

@jvillafanez adjusted to content review

@mrow4a mrow4a force-pushed the feature/persistent-major-versions-v2 branch from efb3480 to 6dfb9e9 Compare February 10, 2023 15:15
@mmattel mmattel force-pushed the feature/persistent-major-versions-v2 branch from 0b09af0 to d38d0fc Compare February 11, 2023 12:34
@mmattel
Copy link
Copy Markdown
Contributor

mmattel commented Feb 11, 2023

I have made a config-to-docs run and fixed the link and text and image size.
Waiting for 40636 in core to be merged.

@mmattel
Copy link
Copy Markdown
Contributor

mmattel commented Feb 11, 2023

no changes in config sample pls.
content comes from core...

@mmattel
Copy link
Copy Markdown
Contributor

mmattel commented Feb 11, 2023

Note, beside the comment from phil-davis above, all is fixed and a language review made. The current build is visible on staging. @mrow4a pls take a look on that comment and respond or fix. Technically we could merge after that.

@phil-davis phil-davis self-requested a review February 12, 2023 04:18
@mrow4a
Copy link
Copy Markdown
Contributor Author

mrow4a commented Feb 13, 2023

as mentioned in #840 (comment) , newly added text is wrong

@mmattel mmattel force-pushed the feature/persistent-major-versions-v2 branch from 9f2fdc8 to c726b7c Compare February 13, 2023 11:00
@mmattel
Copy link
Copy Markdown
Contributor

mmattel commented Feb 13, 2023

as mentioned in #840 (comment) , newly added text is wrong

fixed and rebased, ready to merge

@mrow4a
Copy link
Copy Markdown
Contributor Author

mrow4a commented Feb 13, 2023

your call, I just made doc update suggestion 😉

@mmattel mmattel merged commit e887dd9 into master Feb 13, 2023
@delete-merged-branch delete-merged-branch bot deleted the feature/persistent-major-versions-v2 branch February 13, 2023 11:06
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.

5 participants