Details

Reviewers

bensi
ervin

Commits

R4:9cbfc3d83a73: Handling globalActions gathering from ApplicationComponents instead of main

Summary

Moving the code handling actions gathering to applicationComponenents

Test Plan

unit

Diff Detail

Repository

R4 Zanshin

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

franckarrecot updated this revision to Diff 1394.Nov 25 2015, 7:45 PM

franckarrecot retitled this revision from to Handling globalActions gathering from ApplicationComponents instead of main.

franckarrecot updated this object.

franckarrecot edited the test plan for this revision. (Show Details)

franckarrecot added reviewers: ervin, bensi.

ervin requested changes to this revision.Nov 27 2015, 4:42 PM

ervin edited edge metadata.

ervin added inline comments.

src/widgets/applicationcomponents.cpp
55–56	Perhaps use the methods not the member variables.
57	Nitpick: put that one first or last, just for improving readability.
58	Ditto, use pageView() IMO.
tests/units/widgets/applicationcomponentstest.cpp
432–435	Remove those lines.

This revision now requires changes to proceed.Nov 27 2015, 4:42 PM

Ah also! You need to do the same work in the parts. There's the same action harvesting code there.

updated

ervin requested changes to this revision.Nov 29 2015, 5:19 PM

ervin edited edge metadata.

ervin added inline comments.

tests/units/widgets/applicationcomponentstest.cpp
438	Instead of hard coding the size, it'd probably make sense to check that all the keys() for pageView()->globalActions() are in actions, etc. (for all the sub views). Also add the check on the specific ApplicationComponents action names.

This revision now requires changes to proceed.Nov 29 2015, 5:19 PM

now checking for each entry

ervin requested changes to this revision.Dec 8 2015, 10:07 AM

ervin edited edge metadata.

ervin added inline comments.

tests/units/widgets/applicationcomponentstest.cpp
437	Remove
442–463	Don't hardcode, loop on available->globalActions().keys() verify they are all in actions.
468–471	Ditto.
476–490	Ditto.

This revision now requires changes to proceed.Dec 8 2015, 10:07 AM

bensi requested changes to this revision.Dec 8 2015, 10:26 AM

bensi edited edge metadata.

bensi added inline comments.

src/widgets/applicationcomponents.cpp
52	Should be QHash<QString, QAction*> ApplicationComponents::globalActions() const to be align with the rest of the code, no ?

updated

Minor spacing nitpick left.

src/widgets/applicationcomponents.cpp
52	True that, although we did the mistake at other places, it's no reason to repeat it. :-)

This revision now requires changes to proceed.Dec 8 2015, 11:58 AM

ervin requested changes to this revision.Dec 8 2015, 10:12 PM

ervin edited edge metadata.

ervin added inline comments.

tests/units/widgets/applicationcomponentstest.cpp
440	Damn... I see it only now. :-/ It should be "const auto &key".
445	It should be "const auto &key".
450	It should be "const auto &key".

This revision now requires changes to proceed.Dec 8 2015, 10:12 PM

ervin accepted this revision.Dec 9 2015, 4:10 PM

ervin edited edge metadata.

ervin accepted this revision.Dec 9 2015, 8:51 PM

bensi accepted this revision.Dec 15 2015, 5:09 PM

bensi edited edge metadata.

This revision is now accepted and ready to land.Dec 15 2015, 5:09 PM

ervin edited edge metadata.Dec 15 2015, 9:35 PM

ervin added a project: Zanshin.

Closed by commit R4:9cbfc3d83a73: Handling globalActions gathering from ApplicationComponents instead of main (authored by franckarrecot). · Explain WhyDec 16 2015, 4:53 PM

This revision was automatically updated to reflect the committed changes.

			Path	Packages
M			src/renku/app/main.cpp (6 lines)
M			src/widgets/applicationcomponents.h (4 lines)
M			src/widgets/applicationcomponents.cpp (11 lines)
M			src/zanshin/app/main.cpp (6 lines)
M			src/zanshin/kontact/part.cpp (6 lines)
M			tests/units/widgets/applicationcomponentstest.cpp (26 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	1394	af9fbd7		Nov 25 2015, 7:45 PM	★	★
Diff 2	1417	af9fbd7	updated	Nov 28 2015, 11:56 AM	★	★
Diff 3	1471	f8f5fac	now checking for each entry	Dec 7 2015, 10:16 PM	★	★
Diff 4	1478	f8f5fac	updated	Dec 8 2015, 11:47 AM	★	★
Diff 5	1486	f8f5fac	up	Dec 8 2015, 8:08 PM	★	★
Diff 6	1489	f8f5fac	up	Dec 9 2015, 9:11 AM	★	★
Diff 7	1542	217ff33	Commit rZANSHIN9cbfc3d83a73295fc64aaf57432efa58731cd5a3	Dec 16 2015, 4:41 PM	★	★

Handling globalActions gathering from ApplicationComponents instead of main
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 1542

src/renku/app/main.cpp

src/widgets/applicationcomponents.h

src/widgets/applicationcomponents.cpp

src/zanshin/app/main.cpp

src/zanshin/kontact/part.cpp

tests/units/widgets/applicationcomponentstest.cpp

Handling globalActions gathering from ApplicationComponents instead of mainClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 1542

src/renku/app/main.cpp

src/widgets/applicationcomponents.h

src/widgets/applicationcomponents.cpp

src/zanshin/app/main.cpp

src/zanshin/kontact/part.cpp

tests/units/widgets/applicationcomponentstest.cpp

Handling globalActions gathering from ApplicationComponents instead of main
ClosedPublic
Actions

Revision Contents
Changeset List