Skip to content

Conversation

constantinneagu
Copy link
Contributor

When initializing a new MerginProject, directory must be a str. If we pass something else, like a pathlib.Path, we get a weird error when the logger is initialized.

@tomasMizera tomasMizera requested a review from MarcelGeo July 28, 2025 10:08
Copy link
Contributor

@MarcelGeo MarcelGeo left a comment

Choose a reason for hiding this comment

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

Hi @constantinneagu .

I double-checked what is happening here. Wouldn't it be better to just cast logger_name to a string in the setup_logging method? I think it's just an abstract logger name for the logging library.

str(logger_name)

We are using the os library for manipulating paths in the code. Therefore, we can probably use path-like objects in mergin-client.

Thanks for your contribution, @constantinneagu .

@constantinneagu
Copy link
Contributor Author

Hi @MarcelGeo.

Since I'm not familiar with the code base, I was trying to avoid introducing too many side effects, while at the same time (potentially) sparing a future user from a little bit of frustration. Your solution makes more sense, and it will remove a little bit of weirdness from my side of the code 😄. I'm going to try it and report back.

Copy link
Contributor

@MarcelGeo MarcelGeo left a comment

Choose a reason for hiding this comment

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

If everything is running properly now, thanks for your contribution @constantinneagu

@MarcelGeo MarcelGeo merged commit 341795c into MerginMaps:master Jul 29, 2025
@MarcelGeo MarcelGeo added this to the 0.10.3 milestone Aug 8, 2025
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.

2 participants