Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.89k stars 323 forks source link

How to commit and merge branch retinexadd #3057

Closed Desmis closed 8 years ago

Desmis commented 8 years ago

Hello I'll try to speak in english...I hope no misunderstandings.

1) after Hombre and Sguilder help me to rebuild new environnement with MSYS2, I try to update "reinexadd"

2) in the same time Hombre push a big change, (that works nice !!), and now there are big conflicts

3) as I don't know how to with git...(Git book don't realy help me...the key is hidden in the detail), I made this fusion entirely manually from files that appear in the commit (with the + and - head lines)...big work!

4) I compile and to my surprise ... no errors

5) now I have (on my computer) the code that makes the sum of Retinex (add) and soft proofing

6) I add the new files : git add rtdata.....png and git add tools...

7) and after I want to commit and write : git commit -m "update retinexadd with master 4d6833c" -a

And the system says : Please tell me who you are

On "Msys64" it appears ==> jacques@pc-bureau

I run :

But after that...always the same message : Please tell me who you are run => git config --global user.name and git config --global user.mail

and always I have the same message : fatal : unable to auto-detect email adress (got 'jacques@pc-bureau.(none))

8) If this point is solved, how to merge ? Thank you :)

Desmis commented 8 years ago

I verify on GitHub and: My name = Desmis Public email = jdesmis@gmail.com

When I open Msys64, with Mingw64_shell, it appears jacques@pc-bureau MINGW64 What is wrong ?

Desmis commented 8 years ago

I don't know why ! But I have change nothings...I do the same commands, and now it works, bizarre I try to see if it can be "merge" automatically, but a priori impossible. So how? Thank you for your help :)

Desmis commented 8 years ago

I have solved the conflicts between "Master" and "retinexadd"... Now I'll try to merge ! I hope no error :)

Desmis commented 8 years ago

System says me : "Able to merge" I try to merge and I execute:

1) git checkout master 2) git merge retinexadd I have a message that say in french:

Veuillez entrer un message de validation pour expliquer en quoi cette fusion est
# nécessaire, surtout si cela fusionne une branche amont mise à jour dans une branche de sujet.
#
# Les lignes commençant par '#' seront ignorées, et un message vide
# abandonne la validation.

What to do ??

Desmis commented 8 years ago

Git is really a mystery to me ... to ask me a message! (what message) .. I found an automatic button and it works ... Git may be effective (???), but the least we can say is that the key is hidden in the details. I am not sure, I understood how it works and unsure of being able to start over! I'm probably a french (latin) old (a little stupid) who is struggling to understand the Anglo-Saxon logic

@adamreichold : I did a manual (rawpedia) in French

adamreichold commented 8 years ago

@Desmis Since it has already been merged, I don't think a manual (French or otherwise) will change anything since that would have been a prerequisite for code review which sort of seems to be the point of a pull request?

Desmis commented 8 years ago

@adamreichold : sorry but I'dont really understand what you say ! (a manual for what ?) but no worry, no importance for me...My english is very bad, and I am only a very modest computer :)

Hombre57 commented 8 years ago

Adam a dit :

"Puisque cela a déjà été fusionné dans master, je ne pense pas qu'un manuel (Français ou autre) changera quoique ce soit puisque cela aurait dû être un prérequis à un examen du code qui est normalement l'objet d'un Pull Request".

En gros, pourquoi l'avoir fusionné dans master si on n'a pas la possibilité de vérifier ton patch avant, grâce à la doc qu'il t'avait demandé dans le Pull Request ? Ce serait bien d'attendre la validation de ceux qui s'intéresse à ton PR avant de fusionner. La "nouvelle" politique (qui est celle qu'on aurait toujours dû employer) c'est de ne pousser vers master que des choses stables et finalisée, pour pouvoir en théorie sortir une version Stable Release à n'importe quel moment.

Si tu estimes que personne ne fait de commentaire sur tes patchs, continuons d'utiliser l'ancien système, à savoir prévenir en anglais "si pas d'objection, fusion prévue dans 1 journée" dans le PR lui-même, et être patient.

@adamreichold Is one day enough for waiting for comment on a PR ? I know that you've waited way more on your own PR, but historically, we had a rule saying that if no one comments on the patch within 1 full day (for small patch, probably 2 for bigger patch), the poster could merge. I think this could be discussed again...

adamreichold commented 8 years ago

@Hombre57 If nobody cares, i.e. nobody says something along the lines like I'd like to have a look at that, waiting one or two days and then committing seems reasonable. In this case, I would have liked to invest the time which is why I asked for some introduction or documentation. After all, code review tends to be the most effective way to find problems early, but I personally failed to find a sensible place to start. But never mind, what's merged is merged (or so they say ;-)) and if they are any problems we'll just have to find and fix them later on.

@Desmis This issue looks like it can be closed?

Desmis commented 8 years ago

Hombre Je comprends mieux in french... Mais, il y a un mois (environ ou 3 semaines) DrSlony demandait (in english) pourquoi on ne fusionnait pas retinexadd (Ingo semblait travailler dessus ??), car cela était stabilisé... Cela faisait depuis la mi novembre qu'il n'y avait aucune évolution....Il n'y a pas eu beaucoup de tests, sinon par Paul...Hier matin, j'ai fait la remarque issue3050 (in french) "Je pense d'ailleurs qu'il est possible si personne ne s'y oppose d'en "merge(r)" un (retinexadd)."

Mais, si vous voulez retirer le truc, pour mieux l'évaluer, je n'y vois aucun inconvénient...sinon que je ne sais pas le faire

@adamreichold : oui on peut clore :)

Hombre57 commented 8 years ago

ok, mais il y a eu une demande attachée au PR lui-même. Bref. Je clos.

Beep6581 commented 8 years ago

Hey @Desmis! I'll quote what you wrote:

git config --global user.mail jdesmis@gmail.com

In a shell you must always use quotes to be safe, so it would be safer to write git config --global user.mail "jdesmis@gmail.com"

fatal : unable to auto-detect email adress (got 'jacques@pc-bureau.(none))

The .(none) part of that error makes me think that the dot in your email address was dropped probably because the address wasn't quoted.

1) git checkout master 2) git merge retinexadd I have a message that say in french:

Veuillez entrer un message de validation pour expliquer en quoi cette fusion est
(...)

Merging makes a commit, and there must be a commit message. You can either write one using git merge -m "bla bla" (remember to quote the message!!!) or you can skip the -m "bla bla" part, and then git will show you the editor which you quoted, and you can write the message right in that editor. The advantage of not using -m and therefore of using the editor is that it is easier to write multi-line messages. That's all.