macgitver / MacGitver

MacGitver - THE swiss army knife for Git!
8 stars 0 forks source link

Great refactoring of "Repository Manager" #6

Closed scunz closed 10 years ago

scunz commented 11 years ago

Note: this branch is currently rebased on top of feature/viewsmenu (in libHeaven and libMacGitverCore)

scunz commented 11 years ago

Obviously this has left the stage of being a "Refactoring to some feature"... Damn why do things always get that large?

antis81 commented 10 years ago

@scunz Does not compile here, sorry. Maybe this one helps:

[ 58%] Building CXX object libMacGitverCore/libMacGitverCore/CMakeFiles/MacGitverCore.dir/RepoMan/Repo.cpp.o
/Volumes/Daten/Projects/MacGitver/libMacGitverCore/libMacGitverCore/RepoMan/Repo.cpp:444:17: error: 'aboutToUnload' is a protected member of 'RM::Repo'
        emit r->aboutToUnload(r);
/Volumes/Daten/Projects/MacGitver/libMacGitverCore/libMacGitverCore/RepoMan/Repo.hpp:100:14: note: declared protected here
        void aboutToUnload  (RM::Repo* repo);

Further, cmake tries to link the gtest module, which is not there yet. I outcommented the lines locally to help the linker. I'm curious about these changes :).

scunz commented 10 years ago

gtest is in development branch for a few weeks now.

scunz commented 10 years ago

error: 'aboutToUnload' is a protected member of 'RM::Repo' emit r->aboutToUnload(r);

a. You don't seem to have recent code. Your line numbers don't match mine b. This is a Qt4 vs. Qt5 thing. Did I already say that I don't care for Qt4 anymore? :-)

Will fix this

scunz commented 10 years ago

I'm curious about these changes.

@antis81, you should be afraid of these changes, actually. It is like what I have always said: If we will have this code once, we'll have to almost rewrite everything built upon the core-layer. Which is in fact the most important reason, I refused to implement something in MacGitverModules repository for so long. We need this, but it will change the view to everything :-)

antis81 commented 10 years ago

Not sure, if a complete rewrite is required - if so, I'm not afraid, because it is definitely worth it :). But I simply wait until everything is ready here.

As for Qt4: Thanks for reminding me :). As it is a little tricky to switch back and forth between the two I think I'll remove Qt4 completely and switch over to Qt5 soon.

scunz commented 10 years ago

and switch over to Qt5 soon.

As you're on a Mac, I'd strongly suggest you to wait with that for either 5.1.2 (if that will ever exist) or 5.2.0 to be released. The 5.1.1 is a little messy on Mac OS X. I can give you a few tips, when the stress settled a bit. If you wait till I got my MacBook (Probably not before they release the new ones and Mac OS X 10.9), I will even be able to give you some scripts - Actually, I have them at the customer, but I will need to redo all that work because I obviously can't simply export the work done at the customer...

As it is a little tricky to switch back and forth between the two

Is it?

Now, you can just switch between both by clicking on the "Monitor" Icon on the left of Qt-Creator. When you're on the command line, all of that stuff of course isn't even necessary:

QT5=/where/your/qt5/is/bin

cd /some/place
git clone git@github.com:/macgitver/macgitver.git src
cd src
git submodule init --recursive

cd ..
mkdir bin4
cd bin4
cmake ../src
make 

cd ..
mkdir bin5
cd bin5
PATH=$QT5:$PATH cmake ../src -DQt5=on
make

That should be all...

Though, it's obnoxious that both the PATH=/w/qt/5/bin:$PATH and the -DQt5=On are required. But that is, because we search for the qmake binary and let it give us the correct path to Qt5's cmake files. Actually I've been told a few times that this is wrong and totally f***s up the intention behind that stuff... Well, I still see no better way to use many different Qt5 builds and have this working (Giving the CMAKE_PREFIX_PATH on the command line for the initial cmake run, is much more typing)

Not sure, if a complete rewrite is required - if so, I'm not afraid, because it is definitely worth it

Mostly, it is. Same feelings here :8ball: But when I'm done with this thing and all it's dependencies, it will be incremental work...

scunz commented 10 years ago

Btw.: If you want to try out this code, you should actually clone the Debugging-Addon:

cd $mgv_src/AddOns
git clone git@github.com:/macgitver/MgvDebuggingModules.git

Then rerun cmake and add the RepoTreeRaw view into your hcfg file (~/.local/share/MacGitver/MacGitver/Novice_Novice.hcfg on linux)

scunz commented 10 years ago

Rebased upon both View and Logging changes now, conflicts solved.

SCNR, but: rebasing followed by forced pushing is simply a workflow I don't want to miss: There were 2 conflicts in this code that are now solved for good. Just imagine, this thing would be going on for another 5 months and required other features to be done first... Resolving all the conflicts at the end and probably without even remembering what the concrete topic was 5 months ago...

scunz commented 10 years ago

@antis81, okay, I tried to go the short way and failed: I was thinking that a good logging output would help me sort out the sending of repository events enough to make it half way stable. Instead I'm seeing random events being sent from my new code, so I guess there is no way around creating a set of unit tests and actually be able to say: This code now does exactly what I want it to do.

Sorry, I know that your little spare time on MGV is blocked by this large thing. But I want it rather stable before I get it it... So, it will probably take another 2 to 3 weeks...

antis81 commented 10 years ago

Sorry, I know that your little spare time on MGV is blocked by this large thing. But I want it rather stable before I get it it... So, it will probably take another 2 to 3 weeks...

That's absolutely fine :+1:. I'm sorry I cannot review all of this big thing. But it seems very sane to me and, like said before, I'm excited to see this in action. I also have in mind, that temporarily, features might decrease (like item coloring becoming invisible etc.). These need to be moved to the right place (maybe a delegate or something). But, when the notification system is working I don't worry about these changes too much. Feel free to just move on.

scunz commented 10 years ago

I'm sorry I cannot review all of this big thing.

Well, I didn't expect that for this big thing anyway. Actually, I usually do review my own code very regularly before I deem it "final". But I've indeed given up on this one.

Meanwhile I moved the Logging-Interface-Change and the Config-Refactoring to the bottom of my change sets and once there, just merged them. These changes have been tested partially for 5 weeks, so I'm confident that was a right decision ^^