Solve T6826 +
ClosedPublic

Authored by rizzitello on Feb 18 2018, 9:15 PM.

Details

Summary
  • Solve T6826
  • Clean up the connection dialog.

Changes from refactor independant of the refactor

Diff Detail

Repository
R231 Atelier
Branch
removeSolid
Lint
No Linters Available
Unit
No Unit Test Coverage
rizzitello requested review of this revision.Feb 18 2018, 9:15 PM
rizzitello created this revision.
rizzitello retitled this revision from - Solve T6826 + to Solve T6826 +.Feb 18 2018, 9:17 PM
rizzitello edited the summary of this revision. (Show Details)
rizzitello added a subscriber: Atelier.
rizzitello updated this revision to Diff 27497.Feb 18 2018, 9:29 PM
  • more internal fixes
patrickelectric requested changes to this revision.Feb 18 2018, 9:52 PM
patrickelectric added inline comments.
src/dialogs/connectsettingsdialog.cpp
21

This include should be removed.

24

Same here.

90

Extruder

src/dialogs/connectsettingsdialog.h
37

reference.

src/mainwindow.cpp
136–140

use unique_ptr.

137

get by reference.

This revision now requires changes to proceed.Feb 18 2018, 9:52 PM
rizzitello marked 4 inline comments as done.
  • Patricks suggestions
src/dialogs/connectsettingsdialog.cpp
21

Needed for i18n

  • unique_ptr
rizzitello marked an inline comment as done.Feb 18 2018, 10:47 PM
patrickelectric requested changes to this revision.Feb 18 2018, 11:07 PM
patrickelectric added inline comments.
src/dialogs/connectsettingsdialog.cpp
101

if {
return
}

if {
return
}

src/dialogs/connectsettingsdialog.h
25

This is necessary ?

src/mainwindow.cpp
137

Can you get only core ?

This revision now requires changes to proceed.Feb 18 2018, 11:07 PM

@rizzitello I'll ping the #Solid people about this.

rizzitello marked 3 inline comments as done.
  • further cleanup
src/dialogs/connectsettingsdialog.h
25

Yes but only in the cpp file so i will move it there

src/mainwindow.cpp
137

unfortunately not it needs 'this'

rizzitello added a comment.EditedFeb 18 2018, 11:18 PM

@rizzitello I'll ping the #Solid people about this.

it not so much solid does not work on windows , its more we were not deploying it .. and qt provides us with a system independent way to do this with objects we are already using

patrickelectric accepted this revision.Feb 18 2018, 11:26 PM

Good patch, but before merging it, I would ask for two days after this message to talk with the Solid people about this issue.

This revision is now accepted and ready to land.Feb 18 2018, 11:26 PM

@rizzitello I'll ping the #Solid people about this.

it not so much solid does not work on windows , its more we were not deploying it .. and qt provides us with a system independent way to do this with objects we are already using

We do not deploy Solid, we do not deploy Qt, we do not deploy KStandardAction, KLocalizedString, KActionCollection and other also ! Should we remove all this ? =O

rizzitello updated this revision to Diff 27513.Feb 19 2018, 3:38 AM
  • remove old comment
rizzitello updated this revision to Diff 27516.Feb 19 2018, 3:52 AM
  • Update setting Name for D10648
laysrodrigues accepted this revision.Feb 19 2018, 3:41 PM
laysrodrigues requested changes to this revision.Feb 19 2018, 3:45 PM
laysrodrigues added inline comments.
src/mainwindow.cpp
134

I think this will have a conflict with refactor branch because since I added the feature to connect more than one printer, the _connect action turned in a simple action(without checkable...)

This revision now requires changes to proceed.Feb 19 2018, 3:45 PM
rizzitello marked an inline comment as done.Feb 19 2018, 4:19 PM
rizzitello added inline comments.
src/mainwindow.cpp
134

Refactor will be based on this . There will be changes neeed to refactor as we make these unrelated corrections that need to be made before refactor is merged,

laysrodrigues accepted this revision.Feb 20 2018, 8:38 PM
This revision is now accepted and ready to land.Feb 20 2018, 8:38 PM
rizzitello closed this revision.Feb 20 2018, 8:57 PM