Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actions rebased #6

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Actions rebased #6

wants to merge 34 commits into from

Conversation

stm2
Copy link
Member

@stm2 stm2 commented Aug 29, 2020

rebased #5

@@ -3804,7 +3817,7 @@ void check_money(bool do_move)
if (t->ship == i) {
if (t->hasmoved > 1) { /* schon bewegt! */
sprintf(warn_buf, errtxt[UNITONSHIPHASMOVED], uid(t), itob(i));
Error(warn_buf, t->line_no, t->long_order);
Copy link
Member

Choose a reason for hiding this comment

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

Das hier schient mir der eigentliche Bugfix zu sein, alles andere in diesem commit dient nur der Verwirrung des Reviewers (und macht mir einen Haufen Konflikte beim Cherry-Picking).

Copy link
Member

@ennorehling ennorehling left a comment

Choose a reason for hiding this comment

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

Was war denn der itob buffer bug? von dem steht in Mantis nichts. Ist der unabhängig davon? Kann der dann sein eigener PR sein, bitte?

Copy link
Member

@ennorehling ennorehling left a comment

Choose a reason for hiding this comment

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

importiert, war kompilziert zu picken, weil es help.txt nicht mehr gibt.

Copy link
Member

@ennorehling ennorehling left a comment

Choose a reason for hiding this comment

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

Am Ende habe ich 24 neue commits aus diesem PR extrahiert, und diejenigen kommentiert, die ich nicht mergen kann, und für die ein neuer PR nötig ist.

@@ -4645,6 +4645,13 @@ void help_keys(char key)
int i, j;

switch (key) {
case 'b': /* Befehle */
Copy link
Member

Choose a reason for hiding this comment

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

nicht sicher, warum das gebraucht wird, aber ich habe es übernommen.

@@ -23,7 +23,7 @@ AND, ~and
ASSUMING200STUDYCOSTS, Assuming learnng costs of 200 silver
AWARNING, ~a warning
BEFOREINCOME, before income
BREEDHORSESORHERBS, BREED HORSES or BREED HERBS
Copy link
Member

Choose a reason for hiding this comment

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

Soweit ich sehe, wird diese Meldunge nirgendwo benutzt?

@@ -16,7 +16,7 @@ sawmill
stable
monument
dam
caravanserai
caravanserei
Copy link
Member

Choose a reason for hiding this comment

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

Bitte? Das ist verkehrt. Macht der Server das etwa auch so? Dann sollte es dort gefixt werden.

Copy link
Member

Choose a reason for hiding this comment

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

@@ -87,7 +87,7 @@ void reset_output() {
fclose(OUT);
remove(OUTPUT_FILE);
remove(ERROR_FILE);
set_output(stdout, stderr);
set_output(stdout, stdout);
Copy link
Member

Choose a reason for hiding this comment

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

Die Änderungen an den Tests benötigen einen früheren commit, den ich nicht integriert bekommen habe, deshalb kann ich leider auch diesen nicht integrieren ohne einen neuen PR.

@@ -74,6 +74,7 @@ ISUSEDIN2REGIONS, "TEMP %s" wird in Region %d,%d und Region %d,%d (Zeile %d) geb
ISNOTCOMBATSPELL, k
ITEM, Gegenstand
LINETOOLONG, Zeile zu lang
LOCALEMISMATCH, Locales '%s' von ECheck und '%s' der Befehle passen nicht zusammen
Copy link
Member

Choose a reason for hiding this comment

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

commit wegen Einführung von neuer Message vorerst nicht integriert.

@@ -0,0 +1,150 @@
name: Automatic build and release
Copy link
Member

Choose a reason for hiding this comment

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

github workflows scheint mir ein neues feature zu sein, das ich gerne als eigenen PR gesehen hätte, und deshalb heute überspringe.

@@ -2543,12 +2543,18 @@ void copy_unit(unit * from, unit * to)
to->lives = from->lives;
to->start_of_orders_line = from->start_of_orders_line;
to->long_order_line = from->long_order_line;
if (from->start_of_orders_line)
Copy link
Member

Choose a reason for hiding this comment

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

auch dieser commit ist zu komplex, als das ein einfacher cherry-pick möglich gewesen wäre. Die Änderungen am Test-Framework hätten ein separater PR sein sollen, dann wäre vieles einfacher gewesen.

Copy link
Member

@ennorehling ennorehling left a comment

Choose a reason for hiding this comment

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

Da war ich wohl ein wenig voreilig, das alles so blind zu akzeptieren.

if (*bp == '"') {
quote = (bool) ! quote;
eatwhite = true;
if ((!quote || quote == *bp) && (*bp == DOUBLE_QUOTE || *bp == SINGLE_QUOTE)) {
Copy link
Member

Choose a reason for hiding this comment

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

Diese Änderung musste ich rückgängig machen (commit 4cba246), weil sie nicht mit Kommentaren klar kam, die den Namen einer Einheit wie Patrick O'Connor klar kommt (die also ein einzelnes Anführungszeichen in einem Kommentar haben).

@@ -93,6 +93,7 @@ MISSINGNUMRECRUITS, Anzahl Rekruten fehlt
MISSINGOFFER, Geldgebot fehlt
MISSINGPARAMETERS, Parameter STUFE oder REGION fehlt
MISSINGPASSWORD, Kein Passwort angeben
MISSINGSTART, ERESSEA partei-nummer "Passwort" fehlt
Copy link
Member

Choose a reason for hiding this comment

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

Rückgängig gemacht in commit 417a5e9 weil es nicht damit klar kam, dass in der ersten Zeile ein BOM steht.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants