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

Using Snapper to undo changes #232

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

dariavladykina
Copy link
Contributor

This is the second article from the series that aims to merge and update the content about Snapper from the Admin Guide.

@dariavladykina dariavladykina added the WIP Work in progress. Do not merge! label Nov 27, 2023
<description>For more information</description>
</resource>
<resource xml:id="_snapper-undo-changes-compare-undo-and-rollback" href="../glues/snapper-undo-changes-compare-undo-and-rollback.xml">
<description>For more information</description>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that it matters, but "For more information" isn't the proper description for this glue topic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice topic! The only issue that I have is with the links to non-existing sections or yet-to-be-written articles. Even if they are resolved one day, the disrupt the flow and might make readers jump from this article and never come back.

Only add those links when the reader absolutely needs this reference to proceed with what you're describing. Otherwise put'em right to the end of the article under "For more info".

Copy link
Contributor

Choose a reason for hiding this comment

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

Either keep out of the article until you have something to point to or activate this section and let it point to the respective sections in the legacy docs.

Copy link
Contributor

@janajaeger janajaeger left a comment

Choose a reason for hiding this comment

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

Great job!

As with the first piece, I'm having issues with the sheer abundance of links leading out from the article from pretty much any point. I suggest reviewing every single one of them and trying to determine whether they are needed at that time at all or whether they'd work better at the end.

The article starts a bit out of the blue. It doesn't really have a proper intro that gently states what the thing is all about.

That's about all I could find. Looking forward reading to the entire set of snappery Smart Docs!

Copy link
Contributor

@tomschr tomschr left a comment

Choose a reason for hiding this comment

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

Great article, Daria! 👍 I have some minor issues. Hope they make sense. 🙂

articles/snapper-undo-changes.asm.xml Outdated Show resolved Hide resolved
Comment on lines 35 to 45
<screen> 1 &lt;?xml version="1.0" encoding="utf-8"?&gt;
2 &lt;snapper-zypp-plugin-conf&gt;
3 &lt;solvables&gt;
4 &lt;solvable match="w"<co xml:id="zypp-conf-match"/> important="true"<co xml:id="zypp-conf-important"/>&gt;kernel-*<co xml:id="zypp-conf-kernel"/>&lt;/solvable&gt;
5 &lt;solvable match="w" important="true"&gt;dracut&lt;/solvable&gt;
6 &lt;solvable match="w" important="true"&gt;glibc&lt;/solvable&gt;
7 &lt;solvable match="w" important="true"&gt;systemd*&lt;/solvable&gt;
8 &lt;solvable match="w" important="true"&gt;udev&lt;/solvable&gt;
9 &lt;solvable match="w"&gt;*&lt;/solvable&gt;<co xml:id="zypp-conf-packages"/>
10 &lt;/solvables&gt;
11 &lt;/snapper-zypp-plugin-conf&gt;</screen>
Copy link
Contributor

@tomschr tomschr Dec 12, 2023

Choose a reason for hiding this comment

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

If you add line numbers to a screen you get these disadvantages:

  • Users can't copy and paste your content.
    It leads to invalid XML and the user has to edit it after it has been copied. Very user unfriendly.
  • You does the job of the stylesheets
    There is no need to repeat this tedious task. Delegate it to the stylesheets! 😉 They can do this task.

So please avoid it, don't do it manually.

The recommended way would be this snippet:

<screen linenumbering="numbered">

This would instruct the stylesheets to number each line. However, due to some technical issues, this is currently not supported by xsltproc. 😞

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, it would be nice to see an example with match="re".

concepts/snapper-undo-changes-intro.xml Outdated Show resolved Hide resolved
concepts/snapper-undo-changes-intro.xml Outdated Show resolved Hide resolved
With this configuration snapshot, pairs are made whenever a package is
installed (line 9). When the kernel, dracut, glibc, systemd, or udev packages
marked as important are installed, the snapshot pair will also be marked
as important (lines 4 to 8). All rules are evaluated.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here (line numbers).

glues/snapper-undo-changes-compare-undo-and-rollback.xml Outdated Show resolved Hide resolved
glues/snapper-undo-changes-compare-undo-and-rollback.xml Outdated Show resolved Hide resolved
glues/snapper-undo-changes-compare-undo-and-rollback.xml Outdated Show resolved Hide resolved
tasks/snapper-undo-changes-yast-zypper.xml Outdated Show resolved Hide resolved
tasks/snapper-undo-changes-yast-zypper.xml Outdated Show resolved Hide resolved
@dariavladykina dariavladykina force-pushed the vlada/snapper_2_undo_changes branch from 42dab29 to 00dfa20 Compare January 22, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress. Do not merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants