•  

Comment Results

Review Name Created Custom Fields Content
CR-122 13 Mar 2017

Misschien beter om hier testen te (beginnen) voorzien zoals bij EconNaarApps. M.a.w. gebruik maken van de hier en daar al bestaande StandaardLades.

CR-118 13 Mar 2017

Hiervoor zou ik toch de FormatterAPI gebruiken kwestie dat het toch wel allemaal iets duidelijker leest

CR-118 13 Mar 2017 Ranking: Major Classification: Missing

Waar is de bewaar naar toe ???? ( ofwel moet ik er ergens naast kijken )

CR-118 13 Mar 2017

lap daar gaat onze piloot-klant

CR-118 13 Mar 2017

waarom deze wijziging ??

CR-118 13 Mar 2017

mag het formaat zomaar gewijzigd worden ??

CR-119 13 Mar 2017

In principe is het niet nodig om zulke testen bij te maken. Het is voldoende om één test te hebben die goed overweg kan met de KlantInstellingService:GeefMinimaleBestelhoeveelheid

CR-123 13 Mar 2017

Acceptatie testen niet vergeten

CR-126 13 Mar 2017 Ranking: Major Classification: Extra (superfluous)

??????

CR-126 13 Mar 2017 Classification: Improvement desirable Ranking: Minor

Klasse kan best hetzelfde overervings traject volgen als de TAX equivalent dan is de LBXEnum niet nodig

CR-126 13 Mar 2017

Eerste versie van de test leek mij wel iets duidelijker .. ik zou er wel een extra arrange-assert in zetten

do $$$AssertTrue(LBXKenmerken.NietMeeleveren.Find(Rol)) ; hiermee verzeker je dat de rol er initieel in zat.

Do Actie

do $$$AssertFalse(LBXKenmerken.NietMeeleveren.Find(Rol)) ; hiermee verzeker je dat de rol er uit is.

CR-126 13 Mar 2017

privatiseren ( maar zal uiteindelijk weg mogen ) daar de eerste versie van deze test wel duidelijker was

CR-126 13 Mar 2017

Test is voldoende ipv TestPasAan cfr TAX equivalent

CR-126 13 Mar 2017 Ranking: Major Classification: Risk-prone

Ik zou echter alles testen door de APPS.EC.Winkelkar.impl.ExterneWinkelkarService.KenmerkenAanpasser ipv expliciet de APPS.EC.Winkelkar.impl.ExterneWinkelkarService.KenmerkenAanpasser.LBX aan te roepen.
Nu ontbreken we de stap dat de KenmerkenAanpasser de juiste aanpassing doet indien het om LBX gaat.

CR-118 14 Mar 2017

Ja

CR-118 14 Mar 2017

Verwacht met Elke waarde ?

CR-118 14 Mar 2017

Mock en Indien ??

CR-118 14 Mar 2017

Niet nodig om iedere keer hetzelfde opnieuw te testen..( DRY )

CR-88 14 Mar 2017

Vraagje : Vanwaar de relatie dat 1 leverdatum niet asap is ? Kan 1 leverdatum geen "onmiddellijk leverbaar" betekenen? Of heeft asap een andere betekenis

CR-118 14 Mar 2017

Waarom wordt hier een DummyDto in de constructor meegegeven ?? en wat met de rest van de parameters ?

CR-118 14 Mar 2017

Lijkt mij iets om de vhTest.Utils achter te laten en dat een beetje te algemenen

CR-118 14 Mar 2017

Kwestie van geen $$$ElkeWaarde in mijn VerwachtMethodCall te moeten steken

CR-118 14 Mar 2017

awel, dat ik het eerlijk gezegd zelf totaal niet zou weten. Ik had eerst wat extra wijzigingen in deze klasse (niets van doen met deze lijn, maar soit), maar die zijn allemaal ongedaan gemaakt (behalve die on-the-flight natuurlijk). Zonder die typfout had ik trouwens gewoon een revert gedaan en was er van bovenstaande niets meer te zien. Het was een mysterie, het is een mysterie en zal ten eeuwige dagen een mysterie blijven denk ik.

CR-126 15 Mar 2017

En als ik ze er expliciet bij zet, dan zijn ze waarschijnlijk overbodig.

CR-118 16 Mar 2017 Ranking: Major Classification: Inconsistent

?????

CR-118 16 Mar 2017

!!!!!!

CR-118 17 Mar 2017

Misschien moeten we het iets explicieter maken dat het gaat over "EenBelg" of misschien nog beter "DenBelg" --> natuurlijk zal dat wel het gevolg hebben dat we "DenBelg" als begrip overal moeten gebruiken.
Dan lijkt het mij nog beter om dat "DeBelgischeOnderneming" te nemen

CR-118 17 Mar 2017

Goed idee om die data vanuit een andere Utils te halen

CR-118 17 Mar 2017

Kunnen we het nummer ook niet halen vanuit die andere Utils ???

CR-118 17 Mar 2017

?? Bedoeling van dit veld ? Is een herhaling van de KlantNaam niet ok ???

CR-136 17 Mar 2017 Classification: Risk-prone Ranking: Minor

Geen logica gebruiken in testen .. beter hiervoor een hardcoded waarde voor gebruiken . zal wel te vinden zijn in de vhTest.Utils
Als de ToArray faalt dan zal dit negatieve inpakt hebben op de testen terwijl het er los van staat.

CR-136 17 Mar 2017 Ranking: Minor Classification: Extra (superfluous)

Volgens mij gaat deze test identiek hetzelfde testen dan één van de LijnCacheBuilder testen .. DRY

CR-133 17 Mar 2017 Ranking: Minor Classification: Editorial

Op het eerste zicht lijkt mij dit toch wel een beetje clever code.

En proberen om de $extract in te korten om zo de maximale lengte van een lijn niet te overschrijven dat zal ook wel niet pakken.

Het is beter indien de UItvoeringInfo.DueOutTijdstip een %TimeStamp om de FormatAPI:FormatTimeStamp te gebruiken kwestie van het allemaal toch een beetje leesbaarder te hebben voor de gewone sterveling

CR-133 17 Mar 2017

zie commit msg

CR-139 17 Mar 2017 Ranking: Major Classification: Inconsistent

Appelen zijn geen peren

CR-138 17 Mar 2017 Classification: Improvement desirable Ranking: Minor

NietVerwerkteOrders is good genoeg als variabelen

CR-140 17 Mar 2017 Classification: Improvement desirable Ranking: Minor

En waar staat dat ???

Men moet het gaan zoeken en wat blijkt in de den Assert staat er ergens dat de InterneBreedte 600 is ..
Gelieve de info zo dicht mogelijk te zetten bij de test zelf

CR-140 17 Mar 2017 Ranking: Minor Classification: Editorial

Volgens dat ik kan zien wordt er enkel getest dat Afmeting 75 is .. Geen enkele verwijzing te vinden naar de kleine aangepaste lade. ---> Heb het uiteindelijke gevonden het gaat om die 164 gebruikt onderaan in de test..

Gelieve de nodige aandacht te vestigen om de dingen die er toe doen.

CR-140 17 Mar 2017 Classification: Ambiguous Ranking: Major

Lijkt me wel iets raars dat je ook die PasAan uitvoert als het niet om een spoelbak gaat ?

CR-140 17 Mar 2017 Classification: Ambiguous Ranking: Minor

Gelieve geen setups te verspreiden .. er staat een deel in de testmethod zelf en een deel hier.. Soms kan dat beetje duplicatie echt geen kwaad.

CR-138 20 Mar 2017

en dan op de elvendertig andere plaatsen in deze klasse dat ook veranderen?

en ten andere: is het dan niet beter om er ineens NietVerwerkteOrderIds van te maken?

CR-136 20 Mar 2017

Ik zou dan voorstellen dat je zelf ook geen .ToArray gebruikt in je testen. Commit 39782 Joc en commit 39550 TomV -> vhUnitTest.WSimpl.Vhisie4.Winkelkar.EconConfiguratieConverter.Test

CR-140 20 Mar 2017

Beste de code eens bekijken. Deze werking is eigen aan de werking van de builders.
De aanpasser bepaalt of het om een spoelbaklade gaat en past de kenmerken enkel in dat geval aan.

CR-140 20 Mar 2017

Gebruikte afmetingen zijn naar de test method verhuisd

CR-140 20 Mar 2017

Zoals besproken :
1. Aantal in halffab is een algemene waarde die de lengte representeert en is niet direct gerelateerd aan aantaldwarsverdelingen. Dit veld testen zal dan thuis horen in een andere test die algemener moet zijn.
2. Aangepaste kenmerken, method PasAan geeft geen 2 lades weer, maar 1 lade waardoor de berekening en deze test correct is.

CR-132 22 Mar 2017

Heb je ook de TaorboxModel.Rollen.SetSchroeven aangepast? Dit kan mogelijks fout lopen bij hergenerer

CR-132 22 Mar 2017

Kan in dit geval niet met de standaardlade gewerkt worden?

CR-132 22 Mar 2017

Het hergenereren van deze builder zal opnieuw deze conditie toevoegen. De conditie voor rol SetSchroeven in het TaorboxModel mag verwijderd worden

CR-128 22 Mar 2017 Ranking: Major Classification: Ambiguous

LBX enumeratie maken?
Afwijkende rughoogte nodig?

CR-128 22 Mar 2017

same-o, same-o: tbx enumeratie