Fix macos user_site_dir #137

ThomasWaldmann · 2023-02-05T14:54:20Z

No description provided.

ThomasWaldmann · 2023-02-05T14:54:34Z

Based on #132.

.pre-commit-config.yaml

gaborbernat · 2023-02-05T20:17:52Z

Blocked on https://github.com/platformdirs/platformdirs/pull/132/files

Also: fix a doc string.

- that it doesn't include the version number was not the case (this is unrelated to my change) - it is back to Application Support, so that does not apply any more due to my change

see also 2.0.0 release. :-)

needed to adjust the compatibility-with-appdirs test, because appdirs has the same issue.

ThomasWaldmann · 2023-02-05T20:56:13Z

rebased #132 on fixed main, rebased this on #132.

ThomasWaldmann · 2023-02-06T14:05:09Z

Considering I actually changed something here in the way the paths are made up (removed the trailing slash), I decided to add macOS tests here.

The (IMHO) correct tests added here would have failed in #132 due to the trailing slashes.

tests/test_macos.py

ThomasWaldmann · 2023-02-06T14:20:41Z

tests/test_macos.py

+        # Please note that the config dirs are NOT */Library/Preferences!
+        # For details see: https://github.com/platformdirs/platformdirs/issues/98


guess this should avoid somebody changing this back to the wrong dirs again.

Can you remove this comment? The test itself is enough with the git blame.

Considering this was initially correct, then broken for platformdirs 2.0.0 and now back in the initial correct state, I would strongly suggest we keep that comment, so that this never happens again.

Please remove.

I disagree and I'ld appreciate another persons opinion here.

You can't just assume that the next guy who breaks this will do git blame or a lot of other research first. Otherwise the breakage at 2.0.0 would also not have happened.

And if such a comment would have been there before the 2.0.0 breakage, we would not have had all 2.x.x releases wrong on macOS and also would not have had 2 incompatible/breaking releases in total for macOS users.

Look, the maintainer's opinion trumps your opinion 😊 at the end of the day.

gaborbernat · 2023-02-06T15:18:52Z

tests/test_macos.py

+        # Please note that the config dirs are NOT */Library/Preferences!
+        # For details see: https://github.com/platformdirs/platformdirs/issues/98


Can you remove this comment? The test itself is enough with the git blame.

CHANGES.rst

gaborbernat · 2023-02-06T20:20:31Z

tests/test_macos.py

+        # Please note that the config dirs are NOT */Library/Preferences!
+        # For details see: https://github.com/platformdirs/platformdirs/issues/98


Please remove.

ThomasWaldmann · 2023-02-06T20:53:34Z

Oh, it runs the macOS tests also on windows (and likely also on every other OS). I'll fix that.

Signed-off-by: Bernát Gábor <[email protected]>

ThomasWaldmann · 2023-02-06T21:54:35Z

tests/test_macos.py

-    suffix_elements = []
-    if "appname" in params:
-        suffix_elements.append(params["appname"])
-    if "version" in params:
-        suffix_elements.append(params["version"])
-    if suffix_elements:
-        suffix_elements.insert(0, "")
-    suffix = "/".join(suffix_elements)


BTW, that was c&p from another test module. So guess you want to "fix" it there, too.

ThomasWaldmann · 2023-02-06T22:00:36Z

@gaborbernat This PR is about:

Fixing macOS *_config_dir

(see your edit of the title)

glensc · 2023-02-07T13:31:11Z

Please fill the PR body to this PR, description, related issues, references, etc.

...Since this PR points to the BREAKING CHANGE in dependabot updates,
Well... technically Changelog doesn't point to anything, had to dig up link from Release Notes block.

It's unclear to me what was the BEFORE value? change is just removing of trailing slash? if so, why is trailing slash removal a breaking change?

ThomasWaldmann requested review from Julian, RonnyPfannschmidt, gaborbernat and ofek as code owners February 5, 2023 14:54

ThomasWaldmann changed the title ~~Fix trailing slash (macos)~~ Fix trailing slash (macOS) Feb 5, 2023

gaborbernat reviewed Feb 5, 2023

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

ThomasWaldmann added 4 commits February 5, 2023 21:41

*_config_dir -> "*/Library/Application Support" (macOS), fixes #98

9782dd6

Also: fix a doc string.

remove the outdated test skips for macOS

528760c

- that it doesn't include the version number was not the case (this is unrelated to my change) - it is back to Application Support, so that does not apply any more due to my change

add CHANGES.rst entry

2e2a00b

see also 2.0.0 release. :-)

cleanup trailing slashes (macOS), fixes #134

df86ca9

needed to adjust the compatibility-with-appdirs test, because appdirs has the same issue.

ThomasWaldmann changed the title ~~Fix trailing slash (macOS)~~ Fix trailing slash, add tests, simplify (macOS) Feb 6, 2023

ThomasWaldmann commented Feb 6, 2023

View reviewed changes

tests/test_macos.py Outdated Show resolved Hide resolved

ThomasWaldmann mentioned this pull request Feb 6, 2023

*_config_dir -> "*/Library/Application Support" (macOS), fixes #98 #132

Closed

ThomasWaldmann added 2 commits February 6, 2023 15:17

add macOS tests

496ada1

simplify code: config dir is the same as the data dir (macOS)

fe50a62

ThomasWaldmann changed the title ~~Fix trailing slash, add tests, simplify (macOS)~~ Remove trailing slashes, add tests, simplify (macOS) Feb 6, 2023

ThomasWaldmann commented Feb 6, 2023

View reviewed changes

gaborbernat requested changes Feb 6, 2023

View reviewed changes

gaborbernat reviewed Feb 6, 2023

View reviewed changes

CHANGES.rst Outdated Show resolved Hide resolved

gaborbernat reviewed Feb 6, 2023

View reviewed changes

CHANGES.rst Outdated Show resolved Hide resolved

ThomasWaldmann added 2 commits February 6, 2023 20:52

add CHANGES.rst entry

6f0f0d6

bump date

520f670

gaborbernat changed the title ~~Remove trailing slashes, add tests, simplify (macOS)~~ Fix macos user_site_dir Feb 6, 2023

gaborbernat requested changes Feb 6, 2023

View reviewed changes

skip macOS-specific tests if not on macOS

63d035c

gaborbernat added 2 commits February 6, 2023 13:19

PR feedback

9f6b584

Signed-off-by: Bernát Gábor <[email protected]>

PR feedback

f924ca5

Signed-off-by: Bernát Gábor <[email protected]>

gaborbernat approved these changes Feb 6, 2023

View reviewed changes

gaborbernat merged commit 4fcbe17 into tox-dev:main Feb 6, 2023

ThomasWaldmann commented Feb 6, 2023

View reviewed changes

ThomasWaldmann deleted the fix-trailing-slash-macos branch February 6, 2023 21:56

This was referenced Feb 6, 2023

inconsistent usage of trailing backslashes #134

Closed

macOS user_config_dir goes against OS guidelines #98

Closed

glensc mentioned this pull request Feb 27, 2023

Bump platformdirs from 2.6.2 to 3.0.0 Taxel/PlexTraktSync#1404

Merged

		# Please note that the config dirs are NOT */Library/Preferences!
		# For details see: https://github.com/platformdirs/platformdirs/issues/98

Uh oh!

Fix macos user_site_dir #137

Fix macos user_site_dir #137

Uh oh!

Conversation

ThomasWaldmann commented Feb 5, 2023

Uh oh!

ThomasWaldmann commented Feb 5, 2023

Uh oh!

Uh oh!

gaborbernat commented Feb 5, 2023

Uh oh!

ThomasWaldmann commented Feb 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThomasWaldmann commented Feb 6, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasWaldmann Feb 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasWaldmann commented Feb 6, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasWaldmann commented Feb 6, 2023

Uh oh!

glensc commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ThomasWaldmann commented Feb 5, 2023 •

edited

Loading

ThomasWaldmann Feb 6, 2023 •

edited

Loading

glensc commented Feb 7, 2023 •

edited

Loading