Elliot, Belen Since you both had recommendations, I've implemented both sets of recommendations. The v2 vs v1 changes are attached below the review. The full patch set for 9121 will be emailed shortly.
jshint: no issues in changed segments Thanks for the review Dave > -----Original Message----- > From: Smith, Elliot [mailto:[email protected]] > Sent: Thursday, March 17, 2016 7:45 AM > To: Lerner, Dave > Cc: [email protected]; Belen Barros Pena (Intel); WOOD, MICHAEL > Subject: Re: [Toaster] [PATCH 1/3] toaster: show full list of dependents to > remove > > Thanks for this Dave. Comments inline below. > > On 11 March 2016 at 16:29, Dave Lerner <[email protected]> wrote: > > > When a package is to be removed, show the full list of packages that are > dependent on that package, telling user that these packages will also be > removed. > > [YOCTO #9121] > > Signed-off-by: Dave Lerner <[email protected]> > --- > .../toaster/toastergui/static/js/customrecipe.js | 57 > +++++++++++++++++++--- > 1 file changed, 51 insertions(+), 6 deletions(-) > > diff --git a/bitbake/lib/toaster/toastergui/static/js/customrecipe.js > b/bitbake/lib/toaster/toastergui/static/js/customrecipe.js > index 3c57899..fbb0298 100644 > --- a/bitbake/lib/toaster/toastergui/static/js/customrecipe.js > +++ b/bitbake/lib/toaster/toastergui/static/js/customrecipe.js > @@ -89,7 +89,21 @@ function customRecipePageInit(ctx) { > var depsList = modal.find("#package-reverse-dep-list"); > var deps = pkgData.reverse_dependencies; > > + var depsCount = deps.length; > + var vDepends = "depends"; > + var vPackage = "package"; > + var vThis = "this"; > + if (depsCount > 1) { > + vDepends = "depend"; > + vPackage = "packages"; > + vThis = "these"; > + } > modal.find(".package-to-rm-name").text(targetPkg.name); > + modal.find(".reverse-deps-count").text(depsCount); > + modal.find(".reverse-deps-count-plus1").text((depsCount+1) + " > packages"); > + modal.find(".reverse-deps-depends").text(vDepends); > + modal.find(".reverse-deps-package").text(vPackage); > + modal.find(".reverse-deps-this").text(vThis); > > depsList.text(""); > > @@ -103,6 +117,8 @@ function customRecipePageInit(ctx) { > modal.find("#package-reverse-deps-total-size").text( > pkgData.reverse_dependencies_size_formatted); > > + targetPkg.depsRemoved = deps; > + > rmdPkgReverseDepsModalBtn.data(targetPkg); > modal.modal('show'); > } > @@ -121,6 +137,10 @@ function customRecipePageInit(ctx) { > var btnCell = $("#package-btn-cell-" + targetPkg.id); > var inlineNotify = btnCell.children(".inline-notification"); > > + var i; > + var dep; > + var depBtnCell; > + > if (targetPkg.directive === 'add') { > method = 'PUT'; > /* If the package had dependencies also notify that they were > added */ > @@ -132,15 +152,15 @@ function customRecipePageInit(ctx) { > msg += " packages to " + ctx.recipe.name + ": "; > msg += "<strong>" + targetPkg.name + "</strong> and its > dependencies"; > > - for (var i in targetPkg.depsAdded){ > - var dep = targetPkg.depsAdded[i]; > + for (i in targetPkg.depsAdded){ > + dep = targetPkg.depsAdded[i]; > > msg += " <strong>" + dep.name + "</strong>"; > > /* Add any cells currently in view to the list of cells > which get > * an inline notification inside them and which change > add/rm state > */ > - var depBtnCell = $("#package-btn-cell-" + dep.pk); > + depBtnCell = $("#package-btn-cell-" + dep.pk); > btnCell = btnCell.add(depBtnCell); > > inlineNotify = inlineNotify.add( > @@ -159,9 +179,34 @@ function customRecipePageInit(ctx) { > > } else if (targetPkg.directive === 'remove') { > method = 'DELETE'; > - msg += "removed 1 package from "+ctx.recipe.name+":"; > - msg += ' <strong>' + targetPkg.name + '<strong>'; > - inlineNotify.text("1 package removed"); > + var numPackageString = "1 package "; > + var depCountString = ""; > + if (targetPkg.hasOwnProperty('depsRemoved') && > + targetPkg.depsRemoved.length > 0) { > + numPackageString = (targetPkg.depsRemoved.length + 1) + " > packages"; > > > > I would set a variable numDepsRemoved and use that for all occurrences of > targetPkg.depsRemoved.length in this function, but that's a personal > preference. > > > + depCountString = " and it's " + targetPkg.depsRemoved.length + > " > dependent"; > > Done > "it's" should be "its". > > Done > + if (targetPkg.depsRemoved.length > 1) { > + depCountString += "s"; > + } > + > + for (i in targetPkg.depsRemoved){ > + dep = targetPkg.depsRemoved[i]; > + > + /* Add any cells currently in view to the list of cells > which get > + * an inline notification inside them and which change > add/rm state > + */ > + depBtnCell = $("#package-btn-cell-" + dep.pk); > + btnCell = btnCell.add(depBtnCell); > + > + inlineNotify = inlineNotify.add( > + depBtnCell.children(".inline-notification")); > + } > + } > + msg+= "removed " + numPackageString + " from " + ctx.recipe.name > + ":"; > + msg += " <strong>" + targetPkg.name + "</strong>"; > > + msg += depCountString; > + > + inlineNotify.text(numPackageString + " removed"); > } else { > throw("Unknown package directive: should be add or remove"); > } > -- > 1.9.1 > > -- > _______________________________________________ > toaster mailing list > [email protected] > https://lists.yoctoproject.org/listinfo/toaster > > > > > > -- > > Elliot Smith > Software Engineer > Intel Open Source Technology Centre ====================================== V2 vs V1 diff ====================================== diff --git a/bitbake/lib/toaster/toastergui/static/js/customrecipe.js b/bitbake/lib/toaster/toastergui/static/js/customrecipe.js index fbb0298..f1b8afb 100644 --- a/bitbake/lib/toaster/toastergui/static/js/customrecipe.js +++ b/bitbake/lib/toaster/toastergui/static/js/customrecipe.js @@ -180,17 +180,32 @@ function customRecipePageInit(ctx) { } else if (targetPkg.directive === 'remove') { method = 'DELETE'; var numPackageString = "1 package "; - var depCountString = ""; + var revDepList = ""; if (targetPkg.hasOwnProperty('depsRemoved') && targetPkg.depsRemoved.length > 0) { - numPackageString = (targetPkg.depsRemoved.length + 1) + " packages"; - depCountString = " and it's " + targetPkg.depsRemoved.length + " dependent"; - if (targetPkg.depsRemoved.length > 1) { - depCountString += "s"; + var depsRemovedLength = targetPkg.depsRemoved.length; + var ending = "y: "; + var maxRevDepsDisplayed = 5; + var d = 0; + if (depsRemovedLength > 1) { + ending = "ies: "; } - + numPackageString = (depsRemovedLength + 1) + " packages"; + revDepList = " and its " + depsRemovedLength + " reverse dependenc" + ending; for (i in targetPkg.depsRemoved){ + /* include up to maxRevDepsDisplayed rev deps on the page notification */ + var notShownCount = depsRemovedLength - maxRevDepsDisplayed; dep = targetPkg.depsRemoved[i]; + if (d < maxRevDepsDisplayed) { + if (d > 0) { + revDepList += ", "; + } + revDepList += dep.name; + d++; + if ((d === maxRevDepsDisplayed) && (notShownCount > 0)) { + revDepList += " and " + notShownCount + " more"; + } + } /* Add any cells currently in view to the list of cells which get * an inline notification inside them and which change add/rm state @@ -204,7 +219,7 @@ function customRecipePageInit(ctx) { } msg+= "removed " + numPackageString + " from " + ctx.recipe.name + ":"; msg += " <strong>" + targetPkg.name + "</strong>"; - msg += depCountString; + msg += revDepList; inlineNotify.text(numPackageString + " removed"); } else { -- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
