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

Contribution documentation update #4204

Merged
merged 18 commits into from
Dec 19, 2023

Conversation

DeNic0la
Copy link
Collaborator

@DeNic0la DeNic0la commented Dec 5, 2023

I am so sorry, I messed up something in the git process. I tried to cherry-pick, but that didn't work out how I wanted it to.

This is a continuation of #3874

Everything on this Repo is done, there is still the Documentation and Site I need to update tho.

For your convenience, I would recommend viewing the files in my repo:

README.md

CONTRIBUTING_DE.md

CONTRIBUTING.md

I spellchecked everything. If I missed something, LMK.

Nice to know: Both versions have the same content on the same line. I did this quite consistently, in an attempt to make it easier to keep those files in sync in the future.

Copy link
Member

@manuelmeister manuelmeister left a comment

Choose a reason for hiding this comment

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

Cool, thanks! Quite the effort. 😄

Copy link
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Did another pass. Coming along greatly!

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
1. Fork the main repository onto your GitHub account. To use the commands your configured git `user.name` should be exactly your git user name.
If you run the code below, and it outputs your GitHub username you are good to go.
```shell
echo $(git config user.name)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the echo $() redundant? Wouln't it suffice to run git config user.name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Isn't the echo $() redundant?

Yes

Wouldn't it suffice to run git config user.name?

It would.

But also there is nothing to gain by removing it.

I wrote it that specific way because of line 41:

If not you should replace the $(git config user.name) parts with your username [...]

This part was written for newer developers, it might help them to run a search and replace and it's easier to recognize that this ($(git config user.name)) part refers to the git username if it's always constant.

It doesn't have a huge impact eighter way but I think it brings a slight benefit. I see 0 (real) downsides.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
Weitere Informationen zur Einrichtung einer Entwicklungsumgebung auf deinem Computer findest du im [wiki](https://github.com/ecamp/ecamp3/wiki/installation).
Wenn etwas bei der Einrichtung unklar ist oder du auf einen Fehler gestossen bist, gibt es einen setup-help-Kanal auf [Discord](https://discord.gg/tdwtRytV6P), dort kannst du deine Fragen zum Setup stellen :computer:
### Labels :label:
Aufgaben werden mit Labels gekennzeichnet und einige davon sind nicht selbsterklärend und werden hier erklärt:
Copy link
Member

Choose a reason for hiding this comment

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

Issues

Wenn etwas bei der Einrichtung unklar ist oder du auf einen Fehler gestossen bist, gibt es einen setup-help-Kanal auf [Discord](https://discord.gg/tdwtRytV6P), dort kannst du deine Fragen zum Setup stellen :computer:
### Labels :label:
Aufgaben werden mit Labels gekennzeichnet und einige davon sind nicht selbsterklärend und werden hier erklärt:
- **Good first issue**: :green_heart: Anfängerfreundliche Aufgaben.
Copy link
Member

Choose a reason for hiding this comment

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

Issues

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

WDYM? Willst du Issues gross geschrieben oder möchtest du Aufgaben durch Issues ersetzen?

CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
Copy link
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Found some more typos in the German machine translation

CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
Durch das Prüfen dieser Punkte verbesserst du die Qualität und Konsistenz deiner Beiträge :sparkles: und beschleunigt den Überprüfungsprozess :rocket:.


- [x] **Synchronisation mit dem zentralen Repository:** :arrows_counterclockwise: Stelle sicher, dass dein Fork auf dem neuesten Stand des zentralen Repositories ist, um eine reibungslose Zusammenführung zu ermöglichen. [GitHub-Dokumentation](https://docs.github.com/de/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork)
Copy link
Member

Choose a reason for hiding this comment

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

Statt Zusammenführung würde ich Merge verwenden, oder "Integration deiner Änderungen" falls wir hier primär Git-Anfänger ansprechen

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thematik: Verdeutschung von Anglizismen

Comment on lines 149 to 150
Dieser Datensatz ist darauf ausgerichtet, den Testprozess zu optimieren und eine diverse
Datenabbildung zu erlangen um auch Randfälle zu Testen.
Copy link
Member

Choose a reason for hiding this comment

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

Den zweiten Teil finde ich etwas holprig. Wie wärs mit:

und damit wir beim Entwickeln immer eine breite Pallette an realistischen Daten sowie einige bekannte Edge Cases zu Verfügung haben.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ich habe angepasst auf:

Dieser Datensatz ist darauf ausgerichtet, den Testprozess zu optimieren und damit wir beim Entwickeln immer eine breite Palette an realistischen Daten sowie einige bekannte Randfälle zur Verfügung haben.

Find ich immernoch etwas Holprig. Geht auch in die Thematik "Verdeutschung von Anglizismen"

CONTRIBUTING_DE.md Outdated Show resolved Hide resolved
## Discord :speech_balloon:
Wir verstehen, dass das Einrichten einer Entwicklungsumgebung manchmal knifflig sein kann,
besonders bei unterschiedlichen Systemen und Konfigurationen.
Wenn du auf Probleme stösst oder Hindernisse während der Einrichtung begegnest,
Copy link
Member

Choose a reason for hiding this comment

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

Hindernissen mit n am Schluss

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ich bin mir nicht sicher, ich habe Duden Mentor und ChatGPT gefragt.
image

Bei jedem durchlesen des Satzes werde ich unsicherer.

Da Duuden Mentor mit 'n' auch keinen Fehler findet habe ich es in 'feedback fix XI' hinzugefügt.
Vielleicht kann mich da ja jemand erleuchten.

Copy link
Member

Choose a reason for hiding this comment

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

Ich habe 3 Kolleg*innen befragt: Eine Studentin welche deutsche Sprach- und Literaturwissenschaften studiert, ein Gymnasiallehrer der Deutsch und Geschichte unterrichtet und eine Neulatein-Studentin welche auch schon Styleguides für deutschsprachige Bücher und Artikel verfasst hat. Die Antworten:

Würds i dem Satz au mit 'n' schriebe... Wenns 'auf Hindernisse oder Probleme stösst' heisse würd, dänn ohni aber mit begegnest mit...

Mit n hätti gseit, begegnen verlangt dativ bim Objekt

Mit n und d Satzstellig ändere: „oder während der Einrichtung Hindernissen begegnest“

@BacLuc
Copy link
Contributor

BacLuc commented Dec 17, 2023

You need to merge devel again after #4136

@DeNic0la
Copy link
Collaborator Author

@carlobeltrame Danke für dein Review.
Es gibt noch 4 offene Punkte:
#4204 (comment)
#4204 (comment)
#4204 (comment)
Und das Thema "Verdeutschung von Anglizismen"

Es scheint wir sind uns alle einig dass Inhaltlich gesprochen alles i.O. ist.

Mein Vorschlag für das weitere Vorgehen wäre daher ein Merge.

Danach gibt es 2 Optionen:
A) Du schreibst alles was dir nicht passt um
B) Offene Punkte werden in Person (oder Discord) besprochen

In meinen Augen sind die Punkte die jetzt noch offen sind detail Punkte. Ich habe zu den Punkte keine 'starke Meinung' und ich glaube in Form von Review-Kommentaren darüber zu Diskutieren ist nicht zielführend.

Wenn du damit einverstanden bist, kannst du bitte Mergen?

@carlobeltrame carlobeltrame self-assigned this Dec 18, 2023
@carlobeltrame carlobeltrame force-pushed the contribution-documentation-update branch from 477a392 to 85f35e8 Compare December 19, 2023 11:39
@carlobeltrame carlobeltrame added this pull request to the merge queue Dec 19, 2023
Merged via the queue into ecamp:devel with commit ab1eb29 Dec 19, 2023
31 checks passed
BacLuc added a commit to BacLuc/ecamp3 that referenced this pull request Dec 24, 2023
…jobs in the workflow

That we can use one required status check in the branch protection rules.
This allows to:
- Allows review of the settings which jobs should be required. The merge of a pr leads to the new settings being active
- Not having to synchronize the settings of the required status check with the devel branch.
This happens automatically because the configuration is checked in.
- Not having to merge/rebase devel because the names of the jobs changed but the old/new job
names are in the required status checks.
(see ecamp#4204 (comment))
BacLuc added a commit to BacLuc/ecamp3 that referenced this pull request Dec 24, 2023
…jobs in the workflow

That we can use one required status check in the branch protection rules.
This allows to:
- Allows review of the settings which jobs should be required. The merge of a pr leads to the new settings being active
- Not having to synchronize the settings of the required status check with the devel branch.
This happens automatically because the configuration is checked in.
- Not having to merge/rebase devel because the names of the jobs changed but the old/new job
names are in the required status checks.
(see ecamp#4204 (comment))
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.

4 participants