Project

General

Profile

Bug #2139

Hiv specifik version af bpi-client ind

Added by Thomas Hansen over 2 years ago. Updated 10 months ago.

Status:
Needs code review
Priority:
Normal
Target version:
Estimated time:
URL med eksempel:
Kategorier:
Integration - BPI

Description

Jeg har lige fikset at man ikke kunne redigere det meste indhold på kkb, fordi der var blevet hevet en nyere version af bpi-client ind, og nogen har gjort den inkompatibel med den version af bpi modulet kkb kører med.

Istedet for at hive master ind, skal der en specifik version på her:

https://github.com/ding2/ding2/blob/master/ding2.make

History

#1 Updated by Rolf Madsen over 2 years ago

  • Status changed from New to Needs code review
  • Assignee set to Gitte Barlach
  • Target version set to DDB CMS 2017 1. opgradering (7.x-4.0.2)

#2 Updated by Gitte Barlach over 2 years ago

  • Assignee changed from Gitte Barlach to Jesper Kristensen

#3 Updated by Rolf Madsen over 2 years ago

  • Target version changed from DDB CMS 2017 1. opgradering (7.x-4.0.2) to Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1)

#4 Updated by Jesper Kristensen almost 2 years ago

  • Description updated (diff)
  • Status changed from Needs code review to Reviewed - Needs info/rework
  • Assignee changed from Jesper Kristensen to Gitte Barlach

Der er kun en fejl beskrivelse og ikke noget PR. Så ikke noget at review endnu!

#5 Updated by Gitte Barlach almost 2 years ago

  • Assignee changed from Gitte Barlach to Thomas Hansen

#6 Updated by Thomas Hansen almost 2 years ago

Jeg kan ikke lave et PR, for jeg har ingen anelse om hvad der er "den rigtige" version af BPI at hive ind!

Jeg fiksede det på kkb ved at prøve mig frem og tage en der virkede, men det var mod 2016-2 og der er nok sket noget i mellemtiden.

Nogen af dem her burde kunne klare det: https://github.com/ding2/bpi-client/graphs/contributors

#7 Updated by Rolf Madsen almost 2 years ago

  • Status changed from Reviewed - Needs info/rework to Need more info
  • Assignee changed from Thomas Hansen to Mikkel Ricky

@Mikkel, kan du hjælpe med at afklare Thomas' spørgsmål?

#8 Updated by Mikkel Ricky almost 2 years ago

Jeg er desværre ikke til meget hjælp i denne sag, men umiddelbart virker det fornuftig at ding2 master henter bpi-client master (jf. https://github.com/ding2/ding2/commit/7ec96aabfa1c511344ed7317c6ae41564ee04e66).

Jeg ved heller ikke hvilken version af bpi-modulet kkb kører med. Måske kan Kasper G sige noget fornuftigt?

 

#9 Updated by Mikkel Ricky almost 2 years ago

  • Assignee changed from Mikkel Ricky to Rolf Madsen

#10 Updated by Rolf Madsen almost 2 years ago

  • Assignee changed from Rolf Madsen to Kasper Garnæs

@Kasper, er det noget du kan kaste lys over? :-)

#11 Updated by Simon Holt almost 2 years ago

I release branchen plejer vi at smide en specifik version på: https://github.com/ding2/ding2/blob/7.x-4.0.1/ding2.make#L441

Sådan gør vi også med ting-client.

Når jeg releaser en ny version af vejlebib smider jeg selv en specifik version i make file, som jeg ved virker med vejlebib.

#12 Updated by Rolf Madsen over 1 year ago

  • Target version changed from Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1) to Release 29-2 - Bugfixes (7.x-4.5.0)

#13 Updated by Rolf Madsen over 1 year ago

  • Target version changed from Release 29-2 - Bugfixes (7.x-4.5.0) to Release 29-2 - Bugfixes (Reload)

#14 Updated by Rolf Madsen over 1 year ago

  • Target version changed from Release 29-2 - Bugfixes (Reload) to Release 33 - Bugfixes

#15 Updated by Martin Cording 11 months ago

  • Status changed from Need more info to Rejected

master branch hiver master af modulerne/libraries og releases hiver release version ud.

Det er sådan det bør være.

#16 Updated by Simon Holt 11 months ago

master branch hiver master af modulerne/libraries og releases hiver release version ud.

Det er sådan det bør være.

Enig.

#17 Updated by Rolf Madsen 11 months ago

  • Status changed from Rejected to Needs code review
  • Assignee changed from Kasper Garnæs to Gitte Barlach

@Gitte, kan Core team bekræfte at det er praksis?

#18 Updated by Gitte Barlach 11 months ago

  • Assignee changed from Gitte Barlach to Kasper Garnæs

#19 Updated by Thomas Hansen 10 months ago

master branch hiver master af modulerne/libraries og releases hiver release version ud.

Det er sådan det bør være.

Enig.

Uening.

Når der står i ding2.make at det du har checket ud virker med master fra bpi-client, så er det løgn. Det kan godt være at det virke lige nu, men det er mere held en forstand.

Hvis du sidder på master af ding2 og roder med noget der ikke er bpi relateret, så skal bruge den seneste supportede version, ikke master. Ergo et tag eller sha på bpi-client.

Hvis du roder med bpi-client, så er det dit ansvar at dokumentere hvilken version der er supported. Igen, tag eller sha.

Selvom man sidder på master skal tingene være stabile. Vi kan ikke have at ens udviklings site pludselig whitescreener efter man har buildet, fordi en anden sidder og arbejder med et PR der indfører inkompatible ændringer til et eksternt library (når vi snakker om shared libraries kan de jo også ændre sig udenom DDB CMS).

Det kan godt være at det er besværligt at skulle opdatere makefilen for at få den nyeste version af et ekstert library, men det bør være en eksplicit ting.

#20 Updated by Gitte Barlach 10 months ago

  • Assignee changed from Kasper Garnæs to Jesper Kristensen

#21 Updated by Martin Cording 10 months ago

@Fini:

Men lige netop dit argument gælder jo også for bpi-client modulet.
Her bør master naturligvis være den seneste stabile version.

Hvorfor mener du der er forskel på hvad der skal være stabilt (defacto master) i hhv. ding2 og bpi-client?

#22 Updated by Thomas Hansen 10 months ago

@cording

Så vidt jeg kan bedømme er bpi master *ikke* "seneste stable" der er nogen release branches, så master er vel bleeding edge?

Men selv hvis bpi brugte Git Flow eller et andet branching system hvor master holdes til seneste stable, så er "master" stadig det forkerte at stoppe i ding2.make.

Jeg er sådan set ligeglad med om det er et tag eller et commit sha man peger på, det skal bare være noget der er noget mere entydigt end en branch, for en branch identificerer ikke hvad man mener at en given revision af ding2 fungerer med. At master skal være "seneste stabile version" ændrer ikke på at "seneste stabile version" ikke er specifikt nok. Det ændrer sig nemlig over tid, udenom den kode der er committet til ding2.

 

#23 Updated by Martin Cording 10 months ago

@Fini:

Men det gør ding2:master jo også? Hvorfor mener du at ding2:master er anderledes end bpi-client:master?
Hvis du arbejder på bleeding edge, må du vel også forvente at det kildekode (som din bleeding edge benytter) også er bleeding edge?

En given version af ding2 virker med en given version af andet kildekode.
Hvis du bygger ding2:7.x-4.5.0 får du dét du ønsker; en release (tag) eller specifik sha/revision af alt ekstern kildekode.

#24 Updated by Thomas Hansen 10 months ago

@cording

> Men det gør ding2:master jo også?

Hvor? Vi snakker om ding2's brug af bpi-client, ikke hvordan andre hiver ding2 ind.

> Hvis du bygger ding2:7.x-4.5.0 får du dét du ønsker; en release (tag) eller specifik sha/revision af alt ekstern kildekode.

Og hvis jeg bygger ding2:7.x-4.4.0+3 så falder det hele fra hinanden. Hvilket gør det ret svært at pinpointe en bug på et specifikt commit fordi du kan ikke reproducere buildet som det så ud på det tidspunkt. Du kan heller ikke se hvilket ding2 commit der gjorde det nødvendigt at opgradere et eksternt library.

Det er også inkonsistent, alle andre 3-parts libraries pinner vi til specifikke versioner netop for at ungå den slags problemer som ticketen her beskriver.

#25 Updated by Martin Cording 10 months ago

@Fini:

Hvis du bygger master af ding2, så bygger du en ikke-speficik sha/revision.
Så spørger jeg, hvorfor forventer du at de eksterne kilder der bygges i så fald er specefikke?

Hvis du bygger ding2:7.x-4.4.0+3, så burde du få eksterne kilder af samme release. Det vil sige et tag som peger på en specific sha/revision.
Hvis du ikke gør dét, så mener jeg det er en fejl.

Ift. beskrivelsen af denne ticket, så hører bpi-client sammen med modulet modules/bpi, hvorfor det (ifølge min mening) giver fin mening at releases bliver tagget på den måde som er defineret af Core team.

#26 Updated by Thomas Hansen 10 months ago

@cording

> Hvis du bygger master af ding2, så bygger du en ikke-speficik sha/revision.

Ja, vi er enige om at jeg bygger en ikke-release.

> Så spørger jeg, hvorfor forventer du at de eksterne kilder der bygges i så fald er specefikke?

Fordi jeg forventer at, selv en ikke-release, kan bygges som den så ud på det tidspunkt som den så ud? Ellers kan vi jo bare squashe hele historikken imellem revisioner, for det er jo værdiløst når nyudvikling på en dependecy kan smadre hele sitet.

> Hvis du bygger ding2:7.x-4.4.0+3, så burde du få eksterne kilder af samme release

Nej, for ligesåsnart releasen er lavet bliver versionerne resat til master. Så kan jeg selvfølgelig grave ding2.make frem for 7.x-4.4.0 og kopiere de specifikke tags over, og håbe på at der ikke er nogen ændringer i de commits siden der forventer en nyere revision af deres library end det der var i den seneste release.

Hvis jeg ikke gør det, og der er lavet nogetsomhelst på en af de libraries der kører på master, så...

> Hvis du ikke gør dét, så mener jeg det er en fejl.

Derfor jeg lavede den her ticket.

> Ift. beskrivelsen af denne ticket, så hører bpi-client sammen med modulet modules/bpi, hvorfor det (ifølge min mening) giver fin mening at releases bliver tagget på den måde som er defineret af Core team.

Jeg opponerer ikke imod at noget bliver tagget. Det jeg klager over er at det bliver resat til master imellem releases. Det jeg efterspørger at at versionen på librariet ikke bliver ændret efter release. Hvis du så laver et PR der har krævet ændringer i bpi-client, så retter du det i makefilen til at pege på det commit der er nødvendigt. Så bliver det tydeligt i hvilket commit i dings historik der krævede en opgradering. Så kan man ved ding release tagge det commit i bpi-client med 7.x-4.5.6 der nu passer til releasen og rette makefilen ved release.

#27 Updated by Martin Cording 10 months ago

Jeg må blot erkende mig uenig, idet "master" lige nu (i ding2-regi) fungerer som en slags udviklingsbranch hvor der kan testes på integrationsniveau.

Alternativt kan man følge gitflow yderligere og implementere en "develop" branch til dette formål, og bibeholde master som den seneste stabile release.

#28 Updated by Gitte Barlach 10 months ago

Jeg sender lige denne omkring Core team. 

Also available in: Atom PDF