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