Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Commit

Permalink
RepoMan: use Q_ASSERT instead of assert (fix build on linux)
Browse files Browse the repository at this point in the history
  • Loading branch information
antis81 committed May 13, 2015
1 parent 948f8e9 commit ee2797c
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions Libs/libRepoMan/Data/Repo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

#include "libRepoMan/Data/Repo.hpp"

#include <cassert>

namespace RM
{

Expand All @@ -40,7 +38,7 @@ namespace RM
Git::Repository Repo::gitRepo(bool doLoad)
{
if (!mRepo && doLoad) {
assert(false); // ###REPOMAN - Not sure if we still want to support this...
Q_ASSERT(false); // ###REPOMAN - Not sure if we still want to support this...
}

return mRepo;
Expand Down

2 comments on commit ee2797c

@scunz
Copy link
Member

@scunz scunz commented on ee2797c May 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the code that produced this change from my local repoman. But I'm not sure if that is right.

Since we don't like exceptions, we have at least to ensure that there's no double-bounce when a non-gui thing (i.e. an imaginary headless running test suite) runs into an assert.

But since we might want to (or either might have to) run such an imaginary test suite with enableGui given to the Q{Core}Application constructor, an assert-case might try to fire up a gui... dough.

I.e. remember for instance, that RepoMan has architectural violations: It uses Heaven while it should really be a non-gui thing at all.

@antis81
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't like exceptions, we have at least to ensure that there's no double-bounce when a non-gui thing (i.e. an imaginary headless running test suite) runs into an assert.

Not sure if I completely understand what you want to say with "double bounce" ... You mean like an assertion from two parallel running threads? This problem does exist with exceptions as well, doesn't it? Even worse: They have to be handled separately in each thread, which can produce weird situations ... I'm still convinced we decided for the better - just to mention :).

But since we might want to (or either might have to) run such an imaginary test suite with enableGui given to the Q{Core}Application constructor, an assert-case might try to fire up a gui... dough.

I don't want a GUI based test suite in the first place. This can be done way better with shell script or python maybe. Actually I used that on a daily basis in one of my former work places to test a C# based tool, which got controlled by - surprise - shell script on windows (I'm a naughty guy 😄). The combination is unbeatable. Why? Don't have to care about a UI layout. Need user input (e.g. a custom source-url for a clone service)? Just ask for it on the terminal or setup constants in the test script, providing it to our unit tests. For a test suite to test our Qt/Heaven GUI elements, I'd refer to froglogic - unfortunately it is non-free and I think it would be total overkill for MGV. Not sure if we're talking about the same thing though.

In the first place, the focus lies on finishing the service framework. I'm aware of the fact, that both libraries libMacGitverCore and libActivities do have Heaven dependencies. But, do you think it is worth the effort to split them up in the current state? I suggest to do that in another, separate mile stone.

Please sign in to comment.