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.

Sign up for GitHub

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

Enable more manufacturers and devices #27

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

Enable more manufacturers and devices #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e5ad35
* added Dom77 (Hamm)
christian-weiss Oct 10, 2019
8e41803
* fixed: manufacturers "GL-Inet" and "WD My Net" are now available ag…
christian-weiss Oct 10, 2019
51eb8dd
* fixed: unterminated statement
christian-weiss Oct 10, 2019
e7e5510
* fixed: numbering of keys (manufacturers)
christian-weiss Oct 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
.idea/
.idea/
/*.iml
Copy link

Choose a reason for hiding this comment

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

Das ist IDE-spezifisch. Wollen wir sowas aufnehmen oder sollen die Leute lieber eine persönliche .gitignore pflegen?

Copy link
Member

Choose a reason for hiding this comment

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

Ich glaube es ist einfacher die persönlich zu pflegen. Da unter Umständen für mehrere IDEs Einträge zu Pflegen fände ich irgendwie unschön.

Copy link
Author

Choose a reason for hiding this comment

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

Ist eine Frage der Kosten-Nutzen-Balance.

Vorteil:
a) "Schutz" des Repos vor unnötigem Kram (hilft allen Teammitgliedern)
b) hilft dem Committer ("privates" lokal zu halten; "Dusseligkeitsschutz")

Nachteil:
a) Maximal genauso viele IDE-spezifische Einträge wie es Teammitglieder gibt; mit etwas Glück gibt es Synergien

"Frisst nicht viel Brot" (Mehr Nutzen als Nachteil), daher stimme ich dafür.

Copy link
Author

Choose a reason for hiding this comment

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

Egal wie wir uns entscheiden. Wir sollten es über alle Repos konsistent machen.
Siehe: https://github.com/FreiFunkMuenster/site-ffmsl/pull/2/files#diff-a084b794bc0759e7a6b77810e01874f2R3-R4

Ich stimme für (a). Was meint Ihr @HellMar @MPW1412 ?
Diese Conversation würde ich gerne auf resolved setzen und den PR abschließen.

37 changes: 19 additions & 18 deletions config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,30 +82,31 @@ sites:{
"domaene74" : {id: "domaene74",short:"d74",name:"Recklinghausen", version:"4.1.0", version_base:"2018.2.2", on_map: true},
"domaene75" : {id: "domaene75",short:"d75",name:"Gronau", version:"4.1.0", version_base:"2018.2.2", on_map: true},
"domaene76" : {id: "domaene76",short:"d76",name:"Herne", version:"4.1.0", version_base:"2018.2.2", on_map: true},
"domaene77" : {id: "domaene77",short:"d77",name:"Hamm", version:"4.1.0", version_base:"2018.2.2", on_map: true},
christian-weiss marked this conversation as resolved.
Show resolved Hide resolved
},

//router list for gluon v2018.2
manufacturers: {
"0tp-link": {id: "tp-link", name: "TP-Link"},
"1ubiquiti": {id: "ubiquiti", name: "Ubiquiti"},
"2avm": {id: "avm", name: "AVM"},
"2zyxel": {id: "zyxel", name: "Zyxel"},
"3allnet": {id: "allnet", name: "Allnet"},
"3buffalo": {id: "buffalo", name: "Buffalo"},
"4d-link": {id: "d-link", name: "D-Link"},
"5gl-inet": {id: "gl-inet", name: "GL-Inet"},
"5gl-inet": {id: "gl", name: "GL Innovations"},
"6linksys": {id: "linksys", name: "Linksys"},
"7netgear": {id: "netgear", name: "Netgear"},
"8alfa": {id: "alfa-network", name: "ALFA-Network"},
"98devices": {id: "8devices", name: "8devices"},
"10meraki": {id: "meraki", name: "Meraki"},
"10nexx": {id: "nexx", name: "Nexx"},
"11openmesh": {id: "openmesh", name: "OpenMesh"},
"12onion": {id: "onion", name: "Onion"},
"13wd-ny-net": {id: "wd-ny-net", name: "WD My Net"},
"13wd-ny-net": {id: "raspberrypi", name: "Raspberry Pi"},
"14x86":{id:"x86",name:"x86"}
Comment on lines -100 to -108
Copy link

Choose a reason for hiding this comment

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

Warum wurden diese Klassen entfernt?

Copy link
Author

Choose a reason for hiding this comment

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

Nicht entfernt, nur neu durchnummeriert. (meine Meinung zu diesen Nummern: siehe oben)

Copy link

Choose a reason for hiding this comment

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

Ne, es wurden 17 Zeilen entfernt und nur neun hinzugefügt. Folglich fehlen welche...

Copy link
Author

Choose a reason for hiding this comment

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

17 vorher, 17 nachher = 0 entfernt (14 umbenannt)

"3zyxel": {id: "zyxel", name: "Zyxel"},
"4allnet": {id: "allnet", name: "Allnet"},
"5buffalo": {id: "buffalo", name: "Buffalo"},
"6d-link": {id: "d-link", name: "D-Link"},
"7gl-inet": {id: "gl-inet", name: "GL-Inet"},
"8gl-innovations": {id: "gl", name: "GL Innovations"},
"9linksys": {id: "linksys", name: "Linksys"},
"10netgear": {id: "netgear", name: "Netgear"},
"11alfa": {id: "alfa-network", name: "ALFA-Network"},
Copy link

Choose a reason for hiding this comment

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

Wollen wir nicht einfach nur auf Zahlen umstellen? Diese Nummerierung+Name ist irgendwie komisch.

Copy link
Member

Choose a reason for hiding this comment

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

Dafür. Vermutlich wurde das gemacht weil es so einfacher ist eine Zeile zwischen zu fügen ohne die nachfolgenden aufrutschen zu müssen.

Copy link
Author

Choose a reason for hiding this comment

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

Nur wenn man nummeriert, dann sollte man es auch durchhalten, daher der PR.

Ich finde die Nummerierung grundsätzlich nicht schön, weil sie keinen Mehrwert liefert.
Wenn man neue Werte einfügen möchte der die Reihenfolge der Einträge verändern will, dann ändern muss man, um die aufsteigende Nummerierung durchzuhalten immer x Zeilen aktualisieren. Das erzeugt im Review nur unnötiges Rauschen.

Eigentlich ist es ja nur wichtig das jeder Key eindeutig ist - hierzu würde ich gerne schlicht den Wert aus "id" für den key wiederverwenden/spiegeln. Das würde ich machen, damit der Testaufwand gering ist.

Wenn mal einer Lust das JavaScript-Drumherum anzupassen, dann könnte man das Object aus zu einem Array of Object machen (ohne keys; nur noch Elemente, die aus id+name Objekten bestehen). Das mache ich aber nicht.

Viel wichtiger finde ich aber die User Experience und da wäre es schön, wenn man gemäß "name" alphabetisch sortiert - entweder im Script oder direkt hier in der config. Letztes würde ich machen - damit der Test-Aufwand gering ist.

Was meint Ihr?

Copy link

Choose a reason for hiding this comment

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

Es gibt quasi nur zwei Marken, nah gut inzwischen drei, die verwendet werden: TP-Link, Ubiquiti und AVM. Die sollen oben stehen. Daher haben wir es nicht alphabetisch sortiert sondern diese komischen Zahlen davor gesetzt.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, ok, wenn es um die Sortierung geht, dann wäre es gut, wenn das JavaScript kein "sort by key" macht und einfach die Reihenfolge nimmt, wie sie in der Config vorkommt.

Wenn man das JavaScript aber nicht anpassen möchte (ich finde die Code Stelle gerade nicht), dann wäre ich dafür das zumindest im Keynamen (semantisch) kenntlich zu machen - z.B. position01, position02, etc. bzw. mehr im Streuspeicherverfahren: position0100, position0200, damit man nachträglich Einträge (mittig) ergänzen kann, z.B. position0150, noch später dann position0125 oder pos0175, um nicht bei jeder Einfügung gleich "alle" anderen keys neu durchnummerieren zu müssen.

Copy link
Author

@christian-weiss christian-weiss Oct 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

position: netter Nebeneffekt: führende Nullen würden ansonsten ggf. abgeschnitten
Ich erstelle dafür einen neuen Commit.

"128devices": {id: "8devices", name: "8devices"},
"13meraki": {id: "meraki", name: "Meraki"},
"14nexx": {id: "nexx", name: "Nexx"},
"15openmesh": {id: "openmesh", name: "OpenMesh"},
"16onion": {id: "onion", name: "Onion"},
"17wd-ny-net": {id: "wd-ny-net", name: "WD My Net"},
"18raspberry-pi": {id: "raspberrypi", name: "Raspberry Pi"},
"19x86":{id:"x86",name:"x86"}
},

routers: {
Expand Down Expand Up @@ -315,4 +316,4 @@ name: "Münsterland",

url: "http://firmware.freifunk-muensterland.de/{{parse(downloadableSite).id}}/versions/v{{parse(downloadableSite).version}}/{{selectedMode=='sysupgrade'||parse(selectedRouter).sysupgrade_only=='true'?'sysupgrade':''}}{{parse(selectedRouter).bootloader=='true'&&selectedMode=='factory'?'other':''}}{{parse(selectedRouter).sysupgrade_only!='true'&&parse(selectedRouter).bootloader!='true'&&selectedMode=='factory'?'factory':''}}/gluon-ffms{{parse(downloadableSite).short}}-v" +
"{{parse(downloadableSite).version_base}}+{{parse(downloadableSite).version}}-{{parse(selectedRouter).id}}{{selectedMode=='sysupgrade'||parse(selectedRouter).sysupgrade_only=='true'?'-sysupgrade':''}}{{parse(selectedRouter).bootloader=='true'&&selectedMode=='factory'?'-bootloader':''}}"
}
};
MPW1412 marked this conversation as resolved.
Show resolved Hide resolved
39 changes: 39 additions & 0 deletions shapes/domaene77.geojson
View file
Open in desktop

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions shapes/domaene77_highres.geojson
View file
Open in desktop

Large diffs are not rendered by default.