Closed mathemaat closed 7 years ago
@mathemaat Mooi dat het lijkt te werken! Ik ga het binnenkort bekijken.
De file bevatte veel php requires. Zijn dit (tijdelijke) placeholders, en mogen ze dus weg?
Klopt, zijn placeholders. Die komen nog uit het ontwerp van de webdesigners die in de templates php gebruiken om voor ons aan te geven waar includes nodig zijn. En om de demo php versie te kunnen draaien. Misschien dat we die even allemaal in 1x kunnen vervangen met onze nieuwe (placeholder) includes of html. Dan is ook duidelijk waar code duplication zit, en waar we dat met Django template includes of blocks kunnen oplossen.
Dat "allemaal in 1x vervangen" lijkt me op zich een goed plan. Ik ben er nu min of meer met de hakbijl doorheen gegaan en heb de code specifiek voor dit bestand goed gezet.
Ik wacht je review rustig af. Wij (Jimme en ik) gaan verder met de rest van de issues. Al heeft Jimme (helaas) wel wat problemen met de installatie op zijn nieuwe laptop.
@mathemaat Code ziet er helemaal goed uit!
Wat er nog moet gebeuren op deze pagina zijn de sidebar elementen (categories en twitter). Aangezien de categories sidebar nog niet bestaat kan dat nog niet, en lijkt me dus niet onderdeel van dit item (#162).
Ik vind het overigens (ook) prima om de sidebars als onafhankelijke taak/PR in alle pagina's te zetten. Is niet veel moeite en heeft ook geen invloed op de rest.
Wat mij betreft kunnen we de deze PR mergen. Wat jou betreft ook? Als je wilt, mag je dat zelf doen.
De pagina
templates/pages/blogcategorypage.html
is herschreven volgens de filosofie van Django.Vragen / opmerkingen:
^M
. Dit zou opgelost moeten worden doorgit config --global core.autocrlf true
(op Windows) ofgit config --global core.autocrlf input
(op Linux), maar ik had het idee dat deze commando's allebei niks deden. Ik heb verschillende dingen geprobeerd: reset en add, commit en reset, e.d. Uiteindelijk heb ik maar gewoon gecommit.Laat maar weten of je nog dingen tegenkomt, of dat ik bepaalde denkfouten heb gemaakt.