Re: Review Request: KIO::PreviewJob: Respect the enabled plugins from PreviewSettings
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100578/#review1454 --- Ship it! Looks good, nice cleanup and fix, thanks. Long ago the idea was just if the app doesn't specify plugins, then use them all. But if it's configurable nowadays, then better respect the user's wishes indeed. kio/kio/previewjob.h http://git.reviewboard.kde.org/r/100578/#comment1217 I think this has to be KIO_EXPORT_DEPRECATED instead (for Windows in particular). - David On Feb. 5, 2011, 7:53 p.m., Peter Penz wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100578/ --- (Updated Feb. 5, 2011, 7:53 p.m.) Review request for kdelibs and David Faure. Summary --- Currently KIO::PreviewJob respects the settings MaximumSize and MaximumRemoteSize from the KConfigGroup PreviewSettings, but completely ignores the Plugins settings. Because of this in the following locations the plugins are manually read and passed to KIO::PreviewJob: - Tooltips in Dolphin/Konqueror - Information Panel in Dolphin - KFilePreviewGenerator in kdelibs Recently it turned out that the file-open-dialog ignores the Plugins too. Before adding some code again I've provided this patch for KIO::PreviewJob. Changing the current behavior of KIO::PreviewJob cannot be done, so a new constructor has been added. Following Qt's constructor-pattern the new constructor is quite minimal and setter/getter-methods have been added for the missing parameters. The changed behavior of the new constructor is that if enabledPlugins is zero the plugins from PreviewSettings are used now. If it is OK to merge this patch, I'd take care to replace the calls to the deprecated API within kdelibs + kdebase. Also it might be useful to change the implementation to use PreviewJob::ScaleType internally instead of the two bools bScale and bSave (the ScaleType has been introduced because of the KDE5 comment). But I'd do this in a second commit as it won't change any behavior. Diffs - kio/kio/previewjob.cpp 96e5b27 kio/kio/previewjob.h b86fc9b Diff: http://git.reviewboard.kde.org/r/100578/diff Testing --- Temporary adjusted KFilePreviewGenerator for testing. Thanks, Peter
Re: prison - a barcode library now in kdereview
On Tuesday 15 February 2011, todd rme wrote: On Tue, Feb 15, 2011 at 11:19 AM, Sune Vuorela nos...@vuorela.dk wrote: On 2011-02-15, Parker Coates parker.coa...@kdemail.net wrote: On Tue, Feb 15, 2011 at 03:45, Sune Vuorela wrote: I have added prison (git clone kde:prison) to kdereview, targetting kdesupport. Out of curiosity, what SC component is intended to use it? It is partly based on code I wrote for klipper half a year ago, and I want to replace it with a library. I also plan to make kaddressbook show vcard data in barcode format (so I can transfer it to my phone) I have also heard of someone with ideas for a barcode flake for calligra. Might it also be useful for downloading plasmoids with plasma-mid? eh, why not :) Cheers, Marco Martin
Re: git workflow draft
On Tue, Feb 15, 2011 at 6:51 PM, Aaron J. Seigo ase...@kde.org wrote: Only features / topics that are intended from the state to be merged with master should end up in the main repository, however. More experimental and/or long term efforts (an example presented was the kconfig refactor leading up to 4.0) should start in a personal clone, and when/if the work crosses the border into this is realistically going to be merged with master it can be moved into a branch in the main repository. As far as I'm concerned, the only problem with such branch moves is the potentially epic number of commit notification mails. If so, the email hook should check if the push generates a new branch, and send only one mail then, like 100 commits have been pushed into the new branch foobar; see here for a complete log vs. master and here for the diff. Greetings Stefan
Re: Moving stuff upstream
John Layt wrote: Anyway, I've started a page to document such things at http://community.kde.org/KDE_Core/QtMerge , feel free to add stuff there. David Jarvie, could you add something to the KDateTime entry? Cheers! I added some links to time zone related bug reports. Surprisingly they seem to be moving this week/last week. I say surprisingly because they other day a troll claimed nobody asks for it. It could just be some left hand/right hand stuff I think. steveire ddenis: Are you also working on timezone handling in QLocale? steveire AFAIU that's the reason we have KDateTime instead of QDateTime ddenis steveire: ewww, no :) ddenis steveire: we have briefly talked about timezone info for Qt/QLocale, but didn't come to any conclusion. And nobody asks for it as far as I know steveire I do :) -*- jlayt thinks there's a lot of parts of QLocale and QDateTime that need improving before KDE could consider using them steveire I'm sure I've seen people ask for it on qt intereset and on a bug asking for it steveire jlayt: Agreed. The hard part is convincing nqdf that KDE wanting equates to 'people wanting it' though :) ddenis yeah. But KDE provides lots of things that will never become part of Qt (for example setters in KLocale) steveire Yeah. My motivation is getting the actual functionality in Qt so that we can use Qt APIs like QDateTime in our APIs instead of subclassing and requiring to use KDateTime in our APIs, thereby making KDE libraries incompatible with Qt. steveire Extra setters and convenience methods don't *have* to be in subclasses. steveire just want to get KReplacementClass out of the APIs and get the functionality through a different design. steveire Part of that is getting timezone support in Qt. jlayt ddenis: agreed setters don't belong in QLocale, but we can take care of that ourselves, but I would like to have QLocale support getting / formatting all the features available on all platforms, it would save me a lot of work :-) ddenis jlayt: I don't see it happening :( For example - currency - KLocale just provides too much - we _can_ put it all to Qt, but it will take too much effort with no clear benefit - nobody asking for that. People are asking us to provide a way to format money (number to string), nothing more I can understand nqdf not wanting a code dump from KDE and not wanting to support all of the features KDE does to the extent KDE does, but there's still a lot that could be done in QClasses to enable the broader features required in KClasses. All the best, Steve.
Re: git workflow draft
On Tuesday 15 February 2011 18:32:29 John Layt wrote: Actually, even more complete minutes, outstanding issues and action points can be found at http://community.kde.org/20110213_GitWorkflowAgenda Following up on my action points for the commit template and config settings. I've attached a revised commit template below. The suggestion is for this to be added to each project repo in the root folder to allow for easy distribution and updating. It would also allow each project to customise it as required, e.g. add links to projects own commit policy, extra tags, etc. With regards to recommended git config settings, we could set the repo defaults in .git/config to include things like the kde: shortcuts, the commit template, and possibly push.default none. Do we want to do this? It would mean fewer steps required by a dev to set up their environment, and they can be changed by more skilled devs who don't want them (I'm assuming a locally modified .git/config doesn't get updated from the remote after the initial clone). Any drawbacks? I want to make a start on some of the Git recipe and reference documentation as things occur to me, and was thinking a central http://techbase.kde.org/Development/Git hub page leading off to the various tutorial, policy, recipe, sysadmin, etc pages would be a good idea. Sound OK? Cheers! John. # ---[ You MUST wrap all lines at 72 characters ]--| # # ---[ Please see http://techbase.kde.org/Policies/Commit_Policy ]-| # # ===[ Subject ]===| # ---[ One line only, short meaningful description to show in logs ]---| # ---[ Leave following line blank, do NOT remove ]-| # ===[ Details ]===| # ---[ Describe what has changed and explain why it has changed ]--| # ===[ Fields ]| # ---[ Uncomment and edit as applicable ]--| # ---[ Add Feature to release changelog ] # ---[ Optionally close a wish in bugs.kde.org as implemented ] #FEATURE: optional bug number # # ---[ Close a bug in bugs.kde.org as fixed, with optional version ] #BUG: bug number #FIXED-IN: release version # # ---[ Copy commit message to a bug in bugs.kde.org ] #CCBUG: bug number # # ---[ Copy commit message to an email address ] #CCMAIL: email # # ---[ Close a review in reviewboard.kde.org as submitted ] #REVIEW: review number # # ---[ Advise documentation team of user visible changes in the gui ] #GUI: # # =|
Re: A Qt replacement for KGlobal::ref and deref
On Wed, February 16, 2011 1:44 am, Aaron J. Seigo wrote: On Tuesday, February 15, 2011, Stephen Kelly wrote: http://steveire.com/kdelibs-modular.png * It's broad at the base - Qt developers can pick and choose what they want. There are less interdependencies - you can use the itemviews stuff without also pulling in KLocale KConfig etc. If you're happy with QSettings and QLocale (and Qt developers are happy with those), then you don't use them. * All the platformy stuff is at the top instead of at the bottom. * KDE applications know no difference. The platformy stuff is provided to them still. what's interesting is that we aren't as far from your desired diagram already. your what it looks like now to a qt developer diagram is as much a matter of perspective as it is of the reality. yes, we have modularization to do (the item views, for instance, perhaps being a good example; kdeui has several such things in it), but libkdecore is not such a big issue imho. don't want kconfig? don't use it. splitting it out to its own library is likely to be more burdone that benefit. There would be a major benefit from splitting KConfig etc out of kdecore: Qt developers could use the stripped down library confident in the knowledge that they could use any class in it without having to worry about whether they might accidentally introduce a dependence on platformy stuff. Having all the kdecore classes mixed together in a single library, as now, would mean that Qt developers have to check all the time which classes are safe to use and which aren't - and currently, that isn't even documented clearly. That puts up a big barrier to others using our library. -- David Jarvie. KDE developer. KAlarm author - http://www.astrojar.org.uk/kalarm
Re: git workflow draft
On Wednesday 16 February 2011 10:58:48 John Layt wrote: # ===[ Subject ]===| # ---[ One line only, short meaningful description to show in logs ]---| Personally, I find that starting this short description with the module, library or whatever being touched helps when one is skimming through the log. kzip: Fix typo or kdecore: Fix typo looks better than Fix typo. Is it worth mentioning in the template?
Re: Moving stuff upstream (was: A Qt replacement for KGlobal::ref and deref)
On Wed, February 16, 2011 11:57 am, John Layt wrote: The thing is, most of what is in kdecore date classes really does belong in the Qt classes, these are standard things on all platforms that Qt just hasn't implemented. QLocale is bizarrely incomplete in places. Why wouldn't Qt want to handle timezones correctly in this global internet age? Why wouldn't they want to format dates correctly? Why wouldn't Qt want to support what the system localisation settings actually are? Some of this can actually be fixed in Qt 4.x, we just need to convince Qt to accept the patches. KLocale is something that we need to take a step back from and ask why we have it and if there isn't a better more standard way to achieve those things. We have it so our users can customise their locale settings, to add some non- standard settings, and so apps can change the settings temporarily to do clever things. I've some ideas on how we can do those better. Remove KLocale and the date classes from the core (non-ui) stack and a lot of problems disappear, including their dependency on KConfig. KConfig is going to be the big problem, fortunately not one I deal with :-) Perhaps a KConfig backend to read/write from QSettings files would remove most Qt-only objections to using it in things like kimap? I'd hate to see us have to ifdef support for two config systems in such libraries. Anyway, I've started a page to document such things at http://community.kde.org/KDE_Core/QtMerge , feel free to add stuff there. David Jarvie, could you add something to the KDateTime entry? Done. I hope it contains enough explanation - if you think it needs more, please say so. -- David Jarvie. KDE developer. KAlarm author - http://www.astrojar.org.uk/kalarm
Re: A Qt replacement for KGlobal::ref and deref
On Wednesday, February 16, 2011, David Jarvie wrote: There would be a major benefit from splitting KConfig etc out of kdecore: Qt developers could use the stripped down library confident in the knowledge that they could use any class in it without having to worry about whether they might accidentally introduce a dependence on platformy stuff. it just introduces a new complexity: which of these bazillion libraries do i use and what's the overhead of having so many of them? (not to mention the complications for us actually making that library). QtCore isn't a bazillion libraries either. what i do agree with, however, is that kdecore (as example) does need: * to be properly documented from a higher level than it is now * should have KDE platform features separated out from app dev framework features which begs the question: is KConfig (as ane exmple) platform or app dev? fun conversations to be had and digging to be done :) -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part.
Re: Moving stuff upstream
On Wednesday, February 16, 2011, Stephen Kelly wrote: ddenis jlayt: I don't see it happening :( For example - currency - KLocale just provides too much - we _can_ put it all to Qt, but it will take too much effort with no clear benefit - nobody asking for that. People are asking us to provide a way to format money (number to string), nothing more to me this highlights things we need to work on within the Qt ecosystem: * KDE represents a somebody; or put another way - Qt developers: stop treating one of your largest downstreams as a nobody. to claim nobody is asking for it when we have implemented it and hundreds of applications use it is a bit ... odd :) * if it is too much effort for the core Qt team given their resources, then perhaps we need to find ways to re-purpose some of our efforts in kdelibs (something others have said long before this email, of course :). that requires, however, a working relationship in which all parties feel confident that efforts will not be going to waste, which is a role for open governance to fill * clearer externally visible roadmaps for Qt so we can coordinate and avoid the left hand / right hand thing you observed. another role for open governance. -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Development Frameworks signature.asc Description: This is a digitally signed message part.
KShutdown got to kde-runtime without review
Why? Albert
Re: git workflow draft
On Wednesday, February 16, 2011 13:45:46 Stefan Majewsky wrote: On Tue, Feb 15, 2011 at 6:51 PM, Aaron J. Seigo ase...@kde.org wrote: Only features / topics that are intended from the state to be merged with master should end up in the main repository, however. More experimental and/or long term efforts (an example presented was the kconfig refactor leading up to 4.0) should start in a personal clone, and when/if the work crosses the border into this is realistically going to be merged with master it can be moved into a branch in the main repository. As far as I'm concerned, the only problem with such branch moves is the potentially epic number of commit notification mails. If so, the email hook should check if the push generates a new branch, and send only one mail then, like 100 commits have been pushed into the new branch foobar; see here for a complete log vs. master and here for the diff. This actually came up elsewhere in the meeting as well. There are two major proposed methods on integrating a feature development branch back into master when ready. 1. You can use git-merge in order to merge 2 or more branches into a single commit. The resulting commit is essentially just a normal commit with more than one parent. Because of this, it is possible for git to verify that every commit in history of each parent is present, and more importantly for this problem, there is no duplication of commit emails (since there's only the 1 new commit). A disadvantage is that it can clutter the output of GUI (and CLI) based tools, especially if there are multiple active branches in development at once. 2. You can use git-rebase to basically reapply the diff for every commit in a given branch off of a different base commit. For example: * - A - G - H - I - J - K master \ - B - C - D - E - F solid/make-it-cool In this history tree make-it-cool and master have both diverged from A. If git-rebase were run to integrate make-it-cool into master, the result would look something like: * - A - G - H - I - J - K - B' - C' - D' - E' - F' master Every commit is still there. However, B', C', etc. are logically *new* commits that were created by git-rebase, but with the same commit message (the commit itself would be generated by generating the diff between the original commit and its parent, and then re-applying on the new parent). Since we're generating new commits anyways, it is also possible to clean up the history upon integration (e.g. merge logically related commits C' and D' into C', or split a too-large commit E' into E1' and E2'). However since they are new commits, they would also result in new commit emails being sent, which could be quite the disaster for large branches. In addition some of those re-sent commit emails might try to duplicate previously taken actions for no reason (e.g. a needless CCMAIL: or GUI:). An easy way to solve duplicates is to disable sending commit mails for branches other than master, but I personally dislike that solution as it would result in mailing lists like #kde-commits not receiving any emails until the branch is fully landed on master. (I hate to say it, but imagine a security mistake on ksslsocket.cpp in commit 88 of a 121-commit rebase branch -- is everyone on #kde-commits going to review that deeply into the commit chain?) There may be a way to allow emails on branches and still filter out duplicates upon merging, but the sysadmins (IIRC) didn't know of a way to do it offhand. In addition as Andreas Pakulat mentioned in a response to a rebase-workflow in kde-scm-interest, this completely deletes the fact that the development happened in a branch at all (we could simply retain the old solid/make-it-cool ref so we don't lose that history, but that would make the repository correspondingly larger). A lot of this debate hinges on how we want email-based review to work however (to be clear, kde-commits is *not* the only recipient of email for each git commit, the CIA.vc web service also receives email for each commit). If we want to avoid huge email bombs then we need to either use a merge-based workflow in general, or have some other technical solution to allow for a rebase-based workflow, such as your idea regarding lumping everything into a simple notification email (although this means CIA.vc and possibly things like Commit Filter wouldn't work... :( ) Regards, - Michael Pyne signature.asc Description: This is a digitally signed message part.
Re: Moving stuff upstream (was: A Qt replacement for KGlobal::ref and deref)
John Layt wrote:
On Monday 14 February 2011 22:35:13 Stephen Kelly wrote:
You might think in terms of how much a typical KDE application ends up
using, but I'm thinking in terms of how much a typical Qt application
ends up using. Gregory is not going to end up using KLocale KConfig
KDateTime etc just because he wants to use kimap. I'm thinking in terms
of the Qt community and broader appeal - the people I interact with on
qt-interest, not the KDE community.
The thing is, most of what is in kdecore date classes really does belong
in the Qt classes, these are standard things on all platforms that Qt just
hasn't
implemented. QLocale is bizarrely incomplete in places. Why wouldn't Qt
want
to handle timezones correctly in this global internet age? Why wouldn't
they
want to format dates correctly? Why wouldn't Qt want to support what the
system localisation settings actually are? Some of this can actually be
fixed in Qt 4.x, we just need to convince Qt to accept the patches.
sebsauer has pointed out to me several times that QtMobility has a lot of
useful stuff in it, including handling mimetypes (possible replacement of
the kmimetype stuff?). I also note that the Qt time zone stuff is getting
moved along because of the needs of Qt mobility. I conclude that stuff which
QtMobility needs in Qt get added to Qt, and those things allow QtMobilty to
implement a platform. KDE does not hold that sway. Perhaps it's time to
'align the goals and requirements of QtMobility with those of KDE'.
KLocale is something that we need to take a step back from and ask why we
have
it and if there isn't a better more standard way to achieve those things.
We have it so our users can customise their locale settings, to add some
non- standard settings, and so apps can change the settings temporarily to
do
clever things. I've some ideas on how we can do those better.
Remove KLocale and the date classes from the core (non-ui) stack and a lot
of problems disappear, including their dependency on KConfig.
KConfig is going to be the big problem, fortunately not one I deal with
:-) Perhaps a KConfig backend to read/write from QSettings files would
remove most
Qt-only objections to using it in things like kimap? I'd hate to see us
have to ifdef support for two config systems in such libraries.
I agree that ifdefs shouldn't be used/needed. In extreme cases we can define
an interface to KThing and QThing and have the KThing implementation up on
the platform level. Proof of this concept here:
http://gitorious.org/grantlee/grantlee/blobs/master/templates/lib/abstractlocalizer.h
http://gitorious.org/grantlee/grantlee/blobs/master/templates/lib/qtlocalizer.h
http://websvn.kde.org/trunk/playground/pim/contacts/kdelocalizer.h?view=markup
Although for a config system, I don't think a solution like that would work
in the same way.
I think the real problem is mixed responsibilities of classes and libraries
in kde{,pim}libs. kconfig is respsonible for the file format as well as
de/serializing the data into strings/numbers/bools/dates/whatever. That's an
obvious responsibility of a config system, and it doesn't sound platformy.
However, kconfig is also responsible for knowing where the actual data files
should be, so it depends on KStandardDirs (platformy no doubt). That,
through other interdependencies means that it has the runtime dependencies
on daemons like ktimezoned etc. It doesn't have to be like that.
Could the way to find where the actual files are could be a QStringList,
which is then supplied through the Hollywood principle (don't call us, we'll
call you):
instead of (pseudocode)
(somewhere in kconfig)
KStandardDirs::getConfigFile();
have something like:
someKConfigInstance.setFilePaths(KStandardDirs::getConfigPaths());
The KConfig returned by KGlobal (which would also be in the platformy tier)
would be pre-setup with this.
Or a bit of a virtual interface could be used instead of Hollywood.
The point is that there are many ways of separating functionality from
platform, and it all comes down to what you've heard to be a good idea a
million times: separating responsiblities.
Cheers,
Steve.
Re: Moving stuff upstream (was: A Qt replacement for KGlobal::ref and deref)
David Jarvie wrote: Anyway, I've started a page to document such things at http://community.kde.org/KDE_Core/QtMerge , feel free to add stuff there. David Jarvie, could you add something to the KDateTime entry? Done. I hope it contains enough explanation - if you think it needs more, please say so. I'm not sure the intention should be a wholesale merge of KDateTime into QDateTime or something similar to that. Some of the features and APIs in KThing classes are a direct result of being outside of the Qt APIs. I think the question should be: What is the minimum of changes needed to QDateTime such that it can be used as a data transfer class by KDE? Currently as far as I know, QDateTime simply strips any timezone information, so that's a must. I'm not very familiar with KDateTime but looking at the API, the rest is comparators (we could have a KDateTimeComparator), time delta (daysTo, potentially replacible with http://qt.gitorious.org/qt/qt/merge_requests/1014), and different formats (KDateTimeFormatter?) Or all that stuff could stay in a class called KDateTime, but be repurposed not to be a data transfer class of datetime+timezone info, but a features, functionality and convenience class. If KDateTime were no longer a data transfer class we could use QDateTime in our APIs instead of KDateTime, which would mean our APIs would be compatible with other Qt APIs and therefore usable together. The QDateTime + timezone stuff is in motion right now, so it's a chance to see if it can suit the needs of KDE and not just QtMobility. If you have extra input I'd encourage you to make it on that bug report. http://bugreports.qt.nokia.com/browse/QTMOBILITY-1139
Re: A Qt replacement for KGlobal::ref and deref
Aaron J. Seigo wrote: On Tuesday, February 15, 2011, Stephen Kelly wrote: http://steveire.com/kdelibs-modular.png * It's broad at the base - Qt developers can pick and choose what they want. There are less interdependencies - you can use the itemviews stuff without also pulling in KLocale KConfig etc. If you're happy with QSettings and QLocale (and Qt developers are happy with those), then you don't use them. * All the platformy stuff is at the top instead of at the bottom. * KDE applications know no difference. The platformy stuff is provided to them still. what's interesting is that we aren't as far from your desired diagram already. your what it looks like now to a qt developer diagram is as much a matter of perspective as it is of the reality. yes, we have modularization to do (the item views, for instance, perhaps being a good example; kdeui has several such things in it), but libkdecore is not such a big issue imho. don't want kconfig? don't use it. splitting it out to its own library is likely to be more burdone that benefit. If it were that easy, Gregory wouldn't have forked. He's not the only one either. The MeeGo use of KCal includes forked KDE stuff: http://conference2010.meego.com/session/organise-your-life-calendar-meego http://meego.gitorious.org/meego-middleware/meego-kcalcore Yes, part of the reason is about communicating, part of it is about consequences of using kdecore. There's daemons to be run etc, you're pulling in stuff which you know you don't want to use, like the locale and config stuff and a k plugin system, and you don't know why they exist. there's also libraries like solid, phonon, attica, qca(, akonadi?) which are already in that tier 1 line. even so, we don't communicate that well at all and our build system layout and the social contracts we have with packagers don't help at all. Another thing to remember is that Qt developers wouldn't necessarily use packages from debian or whatever, especially if they're putting together an application or software which they distribute or bundle by their own means. Splitting in packages might be useful from the communication or discoverability angle (Oh! apt-cache tells me libsolid only depends on Qt. I'll use that!), I don't think it helps a lot technically in the appeal to the Qt community. so, good news is that we're closer than one might think from the outside looking in. bad news is we still have quite a bit of work, particularly in kdeui, kio and our build/packaging/communication targets. Yes, it's not as bad as it might look like I'm claiming :), there's a lot of useful changes within reach, or currently just out of reach. I think if no kde library needed to depend on kglobal or any other internal parts of kde which creates interdependencies by nature that would be a huge win. Why does KGlobal::locale() exist, for example? Why isn't that a static method on KLocale? That would mean locale-needing libraries can use that without depending on KConfig. More food for thought.
Re: git workflow draft
Michael Pyne wrote: People who are interested in ksslsocket will see the commits. You're thinking of CommitFilter. I'm thinking of the kde-commits mailing list (i.e. people who didn't *know* they were interested in ksslsocket until they saw a strange commit). I wasn't really. It's just as true for kde-commits. People looking for strange commits are not going to skip over commits just because they're in a batch. I know what you're going to say - They weren't looking for strange commits until they found one :). So why would anyone skip over a bunch of 15 commits in knetwork (or wherever kssl is) just because they're all in one group? It seems to me like getting into purely imaginary issues here. In addition as Andreas Pakulat mentioned in a response to a rebase-workflow in kde-scm-interest, this completely deletes the fact that the development happened in a branch at all (we could simply retain the old solid/make-it-cool ref so we don't lose that history, but that would make the repository correspondingly larger). The assumption that all of the history is useful to have by definition was called into question. I suggested creating useful history so that all history in the release branches is useful. Either way is an assumption, but only one of these assumptions involves deliberately discarding data. ;) What specifically do you mean by creating useful history though? i.e. what should be done additionally in a rebase workflow to get the useful history you refer to? Do this: Another important point I think is that 'topic branches' will not necessarily be normal, but exceptional. Most KDE developers are used to commit early commit often, and that might translate into 'push early, push often', so we'll end up seeing 2 or 3 commit pushes and very few monsters. I rarely push more than 10 commits and very rarely if ever have pushed more than 20. I actually quite agree with you here for the vast majority of topic branches. But it also mitigates one of the disadvantages I mentioned in merging (the cluttered history), as there won't be too many active branches outstanding at any given time, and therefore there shouldn't be large problems with tons of branches at once anyways. I think the most important thing to keep in mind in this whole git workflow discussion is that the first iteration is not going to be correct no matter what. There will need to be changes and adjustments as we go along. Regards, - Michael Pyne
Re: git workflow draft
Sorry, knode fails me again. Some keyboard shortcut must be too close to ctrl+v for me... Stephen Kelly wrote: Either way is an assumption, but only one of these assumptions involves deliberately discarding data. ;) If noise is data, you would have a good point. What specifically do you mean by creating useful history though? i.e. what should be done additionally in a rebase workflow to get the useful history you refer to? Do this: cd /tmp git clone git://gitorious.org/grantlee/grantlee.git cd grantlee git remote add mjansen git://gitorious.org/~mjansen/grantlee/mjansen- grantlee.git git fetch mjansen gitk --all Now look at the commits in the experimental branch from mjansen (before v0.1.5). See that some commits do lots of things at the same time/in the same commit and should be split. There's also commits for s/todo/TODO/, fixing copyrights etc. Having those in separate commits is not 'useful historical data'. There's also a workaround for a bug in the 0.1 branch which I eventually fixed. The 0.1 branch was then merged into experimental and the workaround removed. Speaking as the person who reviewed the branch, this and the large commits issue made it harder to review. I had to navigate around the merges in my head. mjansen might just have been following a 'never rebase public branches' philosphy, but that really doesn't work for me. It was a complicated feature requiring lots of refactoring. When issues crop up I want to know if it was refactoring that introduced the issue, or the feature itself. I don't want to be looking back over the commits 6 months from now, find the workaround, find the merge and removal of the workaround, dig into the 0.1 branch to find out what the actual fix was, find it wasn't relevant to the issue I'm trying to diagnose etc. That's why the information conveyed by the 'actual history' is only noise that increases maintenance burden and reviewability. If your topic branch introduces something and then fixes it 5 commits later, then squash the fix into the introduction and add a commment. A comment is a bazillion times more visible than a hunk among many unrelated hunks in a large commit in a long lived and then merged branch. Look at the history of the actual repo between 0.1.6 and 0.1.7. Small commits. First refactoring to make room for feature, then the feature. It should be impossible to split a commit into two unrelated commits. At least I try to stick to that in grantlee.
Re: git workflow draft
mjansen might just have been following a 'never rebase public branches' philosphy, but that really doesn't work for me. It was a complicated feature requiring lots of refactoring. Hehe ... as the one doing the code i would say it was more like mjansen stumbled through unchartered territory with a blindfold and no clear plan. Or in other words. I knew what i wanted but not the means to do it. So i did try and error while understanding your code. There is no way i could have created a clean history for that branch unless doing it twice. Which more or less happened with you beeing the one doing it twice. Overall i concur with your email. It it just not always possible for mere humans like me. Mike
Re: git workflow draft
Michael Jansen wrote: mjansen might just have been following a 'never rebase public branches' philosphy, but that really doesn't work for me. It was a complicated feature requiring lots of refactoring. Hehe ... as the one doing the code i would say it was more like mjansen stumbled through unchartered territory with a blindfold and no clear plan. Or in other words. I knew what i wanted but not the means to do it. So i did try and error while understanding your code. There is no way i could have created a clean history for that branch unless doing it twice. Which more or less happened with you beeing the one doing it twice. Yes, it's complicated code and a very complicated feature that I would never have figured out. It didn't have to be done twice though. It was written once and committed twice really. The complicated nature of the feature is the reason for it, but the point I was trying to make was that each of the noisy aspects should be discouraged by discouraging merging and encuraging rebasing instead in the documented workflows. I don't want you to think I'm picking on your patches, it's just the set of examples that I have closest to hand :). Thanks, Steve.
