Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.apache.org
Content-Type: multipart/alternative;
 boundary="===============4407519141488145094=="
MIME-Version: 1.0
Subject: Re: Review Request 63083: Add diff viewer to Update Page
From: Stephan Erb <serb@apache.org>
To: Santhosh Kumar Shanmugham <santhoshkumar.s@gmail.com>,
 Reza Motamedi <rmotamedi@twitter.com>, Kai Huang <texasred2013@hotmail.com>
Cc: David McLaughlin <david@dmclaughlin.com>,
 Aurora <reviews@aurora.apache.org>
Date: Tue, 17 Oct 2017 21:03:36 -0000
Message-ID: <20171017210336.5693.44846@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Stephan Erb <noreply@reviews.apache.org>
References: <20171017201119.5693.47562@reviews-vm2.apache.org>
In-Reply-To: <20171017201119.5693.47562@reviews-vm2.apache.org>
Reply-To: Stephan Erb <serb@apache.org>
archived-at: Tue, 17 Oct 2017 21:03:44 -0000

--===============4407519141488145094==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63083/#review188390
-----------------------------------------------------------


Should we consider pretty printing the executor configuration (when assembling the thrift struct in the client)? Hopefully this would yield diffs that are a bit easier to read.

- Stephan Erb


On Oct. 17, 2017, 10:11 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63083/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2017, 10:11 p.m.)
> 
> 
> Review request for Aurora, Kai Huang, Reza Motamedi, and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add diff viewer from Job Page to Update Page. Key difference on update page is that the "after" diff is fixed - it's always the desiredState. But I've extracted the Diff rendering into a shared component between ConfigDiff and UpdateDiff, as well as the CSS.
> 
> 
> Diffs
> -----
> 
>   ui/src/main/js/components/ConfigDiff.js 9627751293ff01c9a3b728e2c352894f1a1e22f3 
>   ui/src/main/js/components/Diff.js PRE-CREATION 
>   ui/src/main/js/components/InstanceViz.js 99efec4c15fcc677820b3145f506997ef8a37207 
>   ui/src/main/js/components/UpdateConfig.js fac3c8856f3fdcb918fe82ac7d61cf0e53b23756 
>   ui/src/main/js/components/UpdateDiff.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/ConfigDiff-test.js 3eaa78a9660e1e4bac284d545f5fae05c24e93f7 
>   ui/src/main/js/components/__tests__/Diff-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateDiff-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js 2764f0e1d6446e5b3aa99d1dc1a952a20efa4798 
>   ui/src/main/sass/app.scss 3a799b658ad6c94144cda7beec38c4b72ec393d8 
>   ui/src/main/sass/components/_diff.scss PRE-CREATION 
>   ui/src/main/sass/components/_job-page.scss 8523fcf4c917b46c9d514325f073bd5788798156 
>   ui/src/main/sass/components/_update-page.scss a0db0b43b61e8ef10c29195945e7543a982aeab1 
> 
> 
> Diff: https://reviews.apache.org/r/63083/diff/1/
> 
> 
> Testing
> -------
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> See screenshots.
> 
> 
> File Attachments
> ----------------
> 
> Selecting from multiple initial configs
>   https://reviews.apache.org/media/uploaded/files/2017/10/17/df437cfa-a97c-4aa1-9a95-2e4291020b9b__Screen_Shot_2017-10-17_at_11.22.18_AM.png
> Showing diff by default
>   https://reviews.apache.org/media/uploaded/files/2017/10/17/740cc0a0-9ac7-413a-8a33-fe452f998400__Screen_Shot_2017-10-17_at_1.09.32_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


--===============4407519141488145094==--