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

[FEATURE][Symbology] Add percantage size unit for Raster Image Marker Layer symbology. #34869

Closed
wants to merge 6 commits into from

Conversation

beketata
Copy link
Contributor

@beketata beketata commented Mar 4, 2020

Description

According to Feature Request this patch adds new percentage (percents of original image size) size unit for Raster Image Marker Layer.

@github-actions github-actions bot added this to the 3.14.0 milestone Mar 4, 2020
Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

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

Nice feature! Are you confident adding a unit test too?

src/core/symbology/qgsmarkersymbollayer.cpp Outdated Show resolved Hide resolved
src/core/symbology/qgsmarkersymbollayer.cpp Outdated Show resolved Hide resolved
src/core/symbology/qgsmarkersymbollayer.cpp Outdated Show resolved Hide resolved
@nirvn
Copy link
Contributor

nirvn commented Mar 5, 2020

@beketata , nice addition. If you want to go the extra mile -> it'd be nice to have the same feature applied to the raster fill symbol.

@beketata
Copy link
Contributor Author

beketata commented Mar 5, 2020

@nirvn , nice addition. If you want to go the extra mile -> it'd be nice to have the same feature applied to the raster fill symbol.

Default value of Image width for Raster image fill is already Original. Or do you mean smth else?

@nirvn
Copy link
Contributor

nirvn commented Mar 5, 2020

@beketata , right, but you could have it at say 50% (of original size) etc.

@beketata
Copy link
Contributor Author

beketata commented Mar 6, 2020

According to tests details Test # 545: PyQgsPostgresRasterProvider is failed.
Could anyone please advise me what to do next? My development environment is MS Visual Studio.

@beketata
Copy link
Contributor Author

beketata commented Mar 6, 2020

I did the same patch adding new percentage size unit for Raster image fill layer.

@beketata
Copy link
Contributor Author

Unfortunately some checks were not successful.
In the details I could see the following info:
Download cygwin Installer
##[error]Cmd.exe exited with code '28'.

Could someone please advise me what to do next?

@roya0045
Copy link
Contributor

Nothing, this test fail as there are some issue with our pipeline, just ignore it.

@stale
Copy link

stale bot commented Mar 26, 2020

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.

@stale stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Mar 26, 2020
@beketata
Copy link
Contributor Author

I received the stale[bot] notification that "this PR has not had any activity ... If there is no further activity ... it will be closed".
What kind of "activity" are you waiting for?

@stale stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Mar 26, 2020
@roya0045
Copy link
Contributor

The tests I would guess.

@beketata
Copy link
Contributor Author

Thank you all for the great support!

@beketata beketata closed this Mar 28, 2020
@Gustry
Copy link
Contributor

Gustry commented Mar 28, 2020

@beketata,

@nyalldawson and @roya0045 were asking if you could add new tests?

@roya0045 told you to not worry about the existing failing test if it's a false positive. (I didn't check)

From the stale message above:

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.

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.

None yet

5 participants