Remove unnecessary overwrite prompt
ClosedPublic

Authored by fakefred on Apr 8 2020, 6:53 AM.

Details

Summary

BUG: 397493

Diff Detail

Repository
R483 Skanlite
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
fakefred requested review of this revision.Apr 8 2020, 6:53 AM
fakefred created this revision.

All unit tests (i.e. one) passed, and I have tested this on all save-as cases I can think of. Thank you for reviewing this patch!

pino added a subscriber: pino.Apr 8 2020, 8:51 AM
  • I'd remove the comment about the removed dialog, as it looks odd when looking at the code without checking the history
  • the statJob is now useless, just drop it

Cool, got it. Will revise that tomorrow.

  • Original Message --------
fakefred updated this revision to Diff 79681.Apr 9 2020, 12:31 AM
  • Remove confusing comments and unnecessary KIO::StatJob
ngraham accepted this revision.Apr 9 2020, 4:02 PM
ngraham edited the summary of this revision. (Show Details)
ngraham added a reviewer: KDE Applications.
ngraham added a subscriber: ngraham.

LGTM. @sars?

This revision is now accepted and ready to land.Apr 9 2020, 4:04 PM
sars accepted this revision.Apr 26 2020, 11:57 AM

Thanks!

Sorry for the faaaar to late response...

I was not sure if this would break the remote protocols (fish/sftp/..), but I just tried it and this patch improves the code thanks!

Please commit! (there where some trailing spaces that you could remove)

off topic:
While testing I found that there is a bug with the remote protocols that the URL is saved wrongly and is broken when you reopen the save dialog...

fakefred updated this revision to Diff 81290.Apr 27 2020, 12:55 AM

Unified changes

Reset previous commits due to incorrect git config, then unified them into a single commit.
Also stripped out the trailing whitespace.

sars accepted this revision.Apr 27 2020, 5:13 AM
cfeck added a subscriber: cfeck.Apr 30 2020, 5:02 PM

Do you have commit rights? If not, please state if the committer's mail address should be the same you used in the bug report, or otherwise state a different address.

No, I do not have commit rights. I actually changed the email on bugzilla recently (but not on phabricator as it seemed impossible to do so w/o sysadmin's manual intervention), so to avoid confusion, it's fkfd@macaw.me. The name should be "Frederick Yin". Thanks!

Thanks! Landing this for you. Nice fix.

This revision was automatically updated to reflect the committed changes.

Also @sars, I notice that Skanlite hasn't gotten a release in a while. I wonder if it would be beneficial to move it to the release service so that it can share the same release schedule as most KDE apps and automatically get three major releases per year, with three minor releases per major one. What do you think?

sars added a comment.May 2 2020, 5:39 AM

Also @sars, I notice that Skanlite hasn't gotten a release in a while. I wonder if it would be beneficial to move it to the release service so that it can share the same release schedule as most KDE apps and automatically get three major releases per year, with three minor releases per major one. What do you think?

Yes please. I do the releases so seldom that I always have to learn again how to do the releases, so then I hesitate to start the release process...

If you go the release service route, can you please wait for the gitlab migration, which should happen soon (two weeks iirc, and way before the cut date for the list of packages in RS 20.08?)

sars added a comment.May 3 2020, 9:09 AM

@ltoscano Yes sure, I have not made a release in over two years :( so waiting a bit more should not be a problem ;)

I should make a manual release soon tho, there is so much new stuff that has been added...