Refactored some casts
ClosedPublic

Authored by carlos_hdc on Oct 11 2018, 3:07 PM.

Details

Summary

Refactored some lines to reduce repeated casts

Diff Detail

Repository
R326 Kalzium
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
carlos_hdc created this revision.Oct 11 2018, 3:07 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptOct 11 2018, 3:07 PM
Restricted Application edited subscribers, added: kde-edu; removed: KDE Edu. · View Herald Transcript
carlos_hdc requested review of this revision.Oct 11 2018, 3:07 PM
filipesaraiva accepted this revision.Oct 11 2018, 6:56 PM
This revision is now accepted and ready to land.Oct 11 2018, 6:56 PM
pino requested changes to this revision.Oct 11 2018, 10:07 PM
pino added a subscriber: pino.

I don't see why this was split from D16120: Changed old style casts -- please move these changed there, and abandon this revision.

This revision now requires changes to proceed.Oct 11 2018, 10:07 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Oct 11 2018, 10:07 PM
Closed by commit R326:06940c77181a: Refactored some casts (authored by carlos_hdc, committed by filipesaraiva). · Explain Why
This revision was automatically updated to reflect the committed changes.
pino added a comment.Oct 11 2018, 10:09 PM

@filipesaraiva are you even reviewing this stuff? OK, let me revert it, so it can be done properly

In D16132#341545, @pino wrote:

@filipesaraiva are you even reviewing this stuff? OK, let me revert it, so it can be done properly

I didn't see the other review request. Pls, go ahead.

Hi @pino, I am here with @carlos_hdc at LaKademy trying to help him. We don't understand what you want. This patch was splitted from D16120 because he was trying to create different reviews for different changes. Were you asking for it, no? If not, please could you detail what you want? Thxs.

pino added a comment.Oct 12 2018, 5:22 AM

This patch was splitted from D16120 because he was trying to create different reviews for different changes. Were you asking for it, no?

In D16120: Changed old style casts I asked to split changes which were not "fix old style casts", and D16140: Removed unreachable lines does exactly that.
This is basic commit hygene:

  • have unrelated changes in different commits, so they can be reviewed & tested more easily
  • document properly what are the changes in a commit

@carlos_hdc started D16120: Changed old style casts (or actually D15690: Removed old style casts, which was abandoned instead of just being fixed...) about "fix old style casts": that is good, so do not lump it with unrelated changes, such as whitespaces changes, remove unreacheable lines, and so forth.

Pino.

I understand that, also please understand that he's a newcomer and he's
still learning the tools and being increasingly difficult to accept a
review (that can be fixed in a follow-up review if the error is not
blatant) is counter productive and demotivating.

We have no commits in kde edu for a while, IV found people to help, let the
commits pass even if he closed the original review and opened a new one.
The code is correct.

Em sex, 12 de out de 2018 às 07:23, Pino Toscano <
noreply@phabricator.kde.org> escreveu:

pino added a comment. View Revision https://phabricator.kde.org/D16132

In D16132#341566 https://phabricator.kde.org/D16132#341566,
@filipesaraiva https://phabricator.kde.org/p/filipesaraiva/ wrote:

This patch was splitted from D16120 https://phabricator.kde.org/D16120
because he was trying to create different reviews for different changes.
Were you asking for it, no?

In D16120: Changed old style casts https://phabricator.kde.org/D16120 I
asked to split changes which were not "fix old style casts", and D16140:
Removed unreachable lines https://phabricator.kde.org/D16140 does
exactly that.
This is basic commit hygene:

apol added a comment.Oct 12 2018, 12:47 PM

Hi Pino,
Please don't revert other people work as unreviewed if it has reviewed by Tomaz or Filipe.

I would prefer solid work being done than 1 perfectly split set of commits and then nothing else.