Refactored some lines to reduce repeated casts
Details
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.
I don't see why this was split from D16120: Changed old style casts -- please move these changed there, and abandon this revision.
@filipesaraiva are you even reviewing this stuff? OK, let me revert it, so it can be done properly
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.
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:
- 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 https://phabricator.kde.org/p/carlos_hdc/ started D16120: Changed old style casts https://phabricator.kde.org/D16120 (or actually D15690: Removed old style casts https://phabricator.kde.org/D15690, 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.
*REPOSITORY* R326 Kalzium
*REVISION DETAIL* https://phabricator.kde.org/D16132
*To: *carlos_hdc, tcanabrava, filipesaraiva, pino *Cc: *pino, kde-edu, narvaez, apol
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.