-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Adding the functionality for specify packaged project filename and title #94
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did interactive review with @SeqLaz
Please fix the tests too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not there.
If you accept the comments from the PR, please accept them in batch instead of individual commits.
Also note that the CI is failing. We need to fix this too, don't overlook it.
Finally, we should also add a test about this.
Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com> Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com> Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com> Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com> Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com> Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com>
5766a85
to
c54da53
Compare
I squashed all the commits into only one commit; after all, there was no need for many changes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting there, please add the title thing to the QgisCoreOffliner
.
Hey @suricactus please have a look. |
libqfieldsync/offline_converter.py
Outdated
export_project_filename = self._export_filename.parent.joinpath( | ||
f"{self._export_filename.stem}.qgs" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export_project_filename = self._export_filename.parent.joinpath( | |
f"{self._export_filename.stem}.qgs" | |
) | |
export_project_filename = self._export_filename |
This is enough, we can add a check earlier that self._export_filename
is a .qgs
file.
This should also be handled in the UI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, now it's mostly styling stuff, all the rest looks good !
Hey @suricactus, in this last commit, I addressed all the suggestions. Have a look! |
This is complement for the PR on QFieldSync