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.

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

Fix #51245 vertex tool topo crs mismatch #58352

Merged

Conversation

Djedouas
Copy link
Member

@Djedouas Djedouas commented Aug 9, 2024

Description

Fixes #51245 where topological editing with vertex tool does not work when layer CRS and project CRS are different.

@github-actions github-actions bot added this to the 3.40.0 milestone Aug 9, 2024
Copy link

github-actions bot commented Aug 9, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 5bf20df)

Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 24, 2024
@Djedouas Djedouas removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 26, 2024
@Djedouas Djedouas changed the title Fix 51245 vertex tool topo crs mismatch Fix #51245 vertex tool topo crs mismatch Aug 27, 2024
@agiudiceandrea agiudiceandrea added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Sep 8, 2024
@Djedouas Djedouas marked this pull request as draft September 9, 2024 07:46
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 24, 2024
@Djedouas Djedouas removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 24, 2024
@nyalldawson
Copy link
Collaborator

@Djedouas is this still a draft? If so, can we please close long-standing draft PRs to keep the queue clean?

@Djedouas
Copy link
Member Author

Djedouas commented Oct 2, 2024

Hi @nyalldawson

I hoped for some help about the test integration…
The code itself does work properly and fixes the issue.

I plan to get back on my PR(s) soon, after having to work on other topics recently.

Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Oct 17, 2024
@Djedouas Djedouas removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Oct 17, 2024
@Djedouas Djedouas marked this pull request as ready for review October 24, 2024 15:25
@Djedouas Djedouas force-pushed the fix-51245-vertex-tool-topo-crs-mismatch branch from 5bf20df to 8785ecf Compare October 24, 2024 15:26
Copy link

github-actions bot commented Oct 24, 2024

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 8785ecf)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 8785ecf)

@Djedouas
Copy link
Member Author

Ready for review!
Unrelated test error.

@Djedouas Djedouas closed this Oct 29, 2024
@Djedouas Djedouas reopened this Oct 29, 2024
Copy link
Member

@lbartoletti lbartoletti left a comment

Choose a reason for hiding this comment

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

tested together, LGTM and a nice simplification. Thanks!

@lbartoletti lbartoletti merged commit c3d5b5e into qgis:master Nov 5, 2024
59 of 60 checks passed
@kannes
Copy link
Contributor

kannes commented Nov 5, 2024

Oh nice, thank you so much!

@Djedouas Djedouas deleted the fix-51245-vertex-tool-topo-crs-mismatch branch November 5, 2024 17:23
@agiudiceandrea
Copy link
Contributor

Since this PR fixes a bug, shouldn't it be backported to both 3.40 and 3.34, or at least to 3.40?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport queued_ltr_backports Queued Backports backport release-3_40 Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Topological editing does not work when project CRS is different than layer CRS
5 participants