Merge 2.5 to dev

[macgills]

No description provided.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (develop@d4d9af2). Click here to learn what that means.
The diff coverage is 35.58%.

@@            Coverage Diff             @@
##             develop    #1192   +/-   ##
==========================================
  Coverage           ?   34.61%           
  Complexity         ?        1           
==========================================
  Files              ?      163           
  Lines              ?     6151           
  Branches           ?      660           
==========================================
  Hits               ?     2129           
  Misses             ?     3808           
  Partials           ?      214

Impacted Files	Coverage Δ	Complexity Δ
.../kiwixmobile/data/local/entity/BookDataSource.java	0% <ø> (ø)	0 <0> (?)
...wixmobile/library/entity/LibraryNetworkEntity.java	33.33% <ø> (ø)	0 <0> (?)
.../kiwix/kiwixmobile/language/LanguagePresenter.java	0% <ø> (ø)	0 <0> (?)
...rg/kiwix/kiwixmobile/language/LanguageAdapter.java	0% <ø> (ø)	0 <0> (?)
...ix/kiwixmobile/data/local/dao/RecentSearchDao.java	57.14% <ø> (ø)	0 <0> (?)
...g/kiwix/kiwixmobile/language/LanguageActivity.java	0% <ø> (ø)	0 <0> (?)
...rg/kiwix/kiwixmobile/data/remote/KiwixService.java	87.5% <ø> (ø)	0 <0> (?)
...xmobile/data/local/entity/NetworkLanguageSpec.java	0% <ø> (ø)	0 <0> (?)
.../java/org/kiwix/kiwixmobile/main/MainContract.java	0% <ø> (ø)	0 <0> (?)
.../org/kiwix/kiwixmobile/data/local/dao/BookDao.java	33.33% <ø> (ø)	0 <0> (?)
... and 97 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4d9af2...5431afa. Read the comment docs.

[macgills]

@mhutti1 I am going to merge this tomorrow unless you have any objections. It is frightfully large but it is mostly what you have seen before and I am a bit blocked and eager to see a dev branch

[soloturn]

@macgills a couple of weeks ago you told me to not look at the commits, because your working style is different, and one should look more at the pull request. fair enough. i appreciate if you jiggle all the balls and make stuff work your way touching all the existing code at once. excellent!

but now it seems you want to merge "try this, try that", "do this, revert this, redo it again", "comment code" style commits into master, correct? as you said so i looked at the pull request as a whole. the only summary i could come up with is "miracle which should make it work".

hey it is not forbidden to spend a little time to beautify the commit history, split it up into parts which do make logical sense. squash commits. invest more than 2 seconds to write a proper commit message. i know, the more mess a commit history is, the longer this takes. but yours does not look messy. how much time you would need @macgills to beautify it?

[kelson42]

@soloturn The only thing which comes to my mind after your comment is: Why not making a PR to improve the CONTRIBUTING.md with some coding guidelines? That would avoid this kind of discussions about a specific PR.

[macgills]

@soloturn this is just the same discussion we had on the original PR, those commits are just here again because I am setting up the new branching model and need to get my changes onto develop.
Virtually all this code has been reviewed before and has already been merged, there won't be a ticket like this again with 100~ commits +7,349 −4,118 if we adhere to the new guidelines.
Also the target is not master and the resulting commit is not throwing these commits onto master, a merge commit will be created.

[soloturn]

aha :) you thnk it is beneficial to add a small paragraph in the lines of:

open source projects works different than a corporate project with respect to commit quality. corporate, the commit is less important, as they tend to loose the history. it is quite regular you employ a person which commits a miracle and walks away later on. for a contractor it may even a success factor to commit in a way nobody else understands it. sometimes lines of code changed are counted. it is very unusual that the commits are checked.

in an open source project this is different. the commit history is dragged along forever. people tend to read the single commits. the motivation to contribute is not money, but learning, fix what bothers you, and other non-monetary drivers. giving such incentives is crucial, otherwise people would walk away.

?

[macgills]

I think reading single commits is only useful in extremely rare scenarios.

An open source project is the same as any other project, it is about delivering a product while adhering to the guidelines of the project.

The new proposed guidelines are "put your stuff on a feature branch and once that feature is complete open a PR".
Master is a history of releases, develop is a history of features, a feature branch is the history of the actual commits that went into a feature.
All history is maintained, it is just orderly and more importantly true.

If you have a proposal for these guidelines then perhaps request changes on #1209 or comment on #1200 , https://nvie.com/posts/a-successful-git-branching-model/ this is the basis for the model and it has become an industry standard https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow

[kelson42]

@macgills @soloturn Considering that this PR has been reviewed and merged. I don't think this a good idea to continue this discussion. There is currently a PR about improving merging quality/documentation #1209. We should focus on that. If we want then to make further guidelines, I'm happy about any new ticket and then PR going in that direction.

[soloturn]

@kelson42 cool, #1209 is closed and merged already :) master had a nice commit quality the last 6 month or so. but now we get substandard commits, without proper code revew. @macgills does not know how to or does not want to squash, and rebase. in an open source project this is a no-go. in my opinion. not even a simple text file with a 10 line change can be done in one commit with a proper commit message any more: https://github.com/kiwix/kiwix-android/pull/1209/commits

what triggered this message which may sound a little harsh. @macgills did not answer my question: how long would it take @macgills to beatify the commit history so it is easy(ier) to read?

[kelson42]

@soloturn

• I don't think neither master nor develop branches log revision will be rewritten. The reason is that (1) other priorities (2) to a large extend, it fits the new "git rules" available in the CONTRIBUTING.md.
• For Kiwix Android repo, rules have been set, and as long as they don't change they should be respected.
• Like I said, this discussion, for the moment, in that form, is leading nowhere. Worse: it seems that it escalates and hurt feelings. I'm sure nobody really wants that. So I ask firmly to stop this dead-end discussion and accept our common failure to get an agreement.
• Each of us agrees about the importance of the topic and that it would be better to reach a consensus. Therefore I have created Setup global Kiwix/openZIM code collaboration rules overview#18. I will work on it with all the good will of people interested to build that new regulation.

[soloturn]

@kelson42 i only want (now) to get a rough estimation how long it takes from @macgills to rewrite the history so we have facts and not guesses.

i love your diplomatic way to create 18 :) i will start to collect some links there.