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.

Sign up for GitHub

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

Jump to bottom

users: New default config/data locations #833

Merged
merged 8 commits into from
Nov 13, 2023
Merged

users: New default config/data locations #833

merged 8 commits into from
Nov 13, 2023

Conversation

calmh
Copy link
Member

@calmh calmh commented Oct 22, 2023

acolomb
acolomb requested changes Oct 22, 2023
View reviewed changes
Copy link
Member

@acolomb acolomb left a comment

Choose a reason for hiding this comment

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

Nicely done, but some corrections necessary.

users/config.rst Outdated Show resolved Hide resolved
users/config.rst Outdated
@@ -48,22 +50,27 @@ located:

The database is stored either in the same directory as the config (usually the
default), but may also be located in one of the following directories (Unix-like
platforms only):
platforms only) for legacy reasons:
Copy link
Member

Choose a reason for hiding this comment

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

We should mention the legacy config location as well, i.e. %XDG_CONFIG_HOME/syncthing and $HOME/.config/syncthing, as they are is still tried.

users/config.rst

:file:`index-{*}.db`
A directory holding the database with metadata and hashes of the files
currently on disk and available from peers.

:file:`syncthing.log`
Copy link
Member

Choose a reason for hiding this comment

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

csrftokens.txt also lands here if I read the code correctly. Move from the paragraph above please.

users/faq-parts/troubleshooting.rst Outdated
@@ -128,7 +128,7 @@ The web GUI contains a ``Recent Changes`` button under the device list which
displays changes since the last (re)start of Syncthing. With the ``--audit``
option you can enable a persistent, detailed log of changes and most
activities, which contains a ``JSON`` formatted sequence of events in the
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Double space. Which is fine after a full-stop (end of sentence), but not within.

Suggested change
activities, which contains a ``JSON`` formatted sequence of events in the
activities, which contains a ``JSON`` formatted sequence of events in the

@acolomb
Copy link
Member

acolomb commented Oct 22, 2023

Oh and we should maybe have a ..versionadded:: tag here somewhere, to make it explicit when the defaults changed.

@calmh
Copy link
Member Author

calmh commented Oct 23, 2023 •
edited
Loading

I rewrote stuff to be, in my mind, a bit clearer. A new versionchanged describes the change. Then we don't need to deal with the change/legacy further down in the doc. I deliberately left out mentioning csrftokens.txt (but missed removing it from the old config section) because it's a tiny implementation wart that should go away (live in the database, if anywhere) -- it's covered by the new "among others" on the database dir. :)

@calmh
Copy link
Member Author

calmh commented Oct 23, 2023

n.b. the versionchanged is next+1 since I think this is too dangerous a change to slide into an rc.2. We should delay merge accordingly (of the docs change), until after 1.26 release.

@calmh calmh merged commit 2c95cf7 into main Nov 13, 2023
5 checks passed
@calmh calmh deleted the xdgstatehome branch November 13, 2023 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acolomb acolomb acolomb requested changes

@imsodin imsodin imsodin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@calmh @acolomb @imsodin