aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santhosh Kumar Shanmugham <santhoshkuma...@gmail.com>
Subject Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme
Date Wed, 27 Sep 2017 20:59:21 GMT

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


Ship it!




LGTM. Minor comments about readability.


ui/src/main/js/components/Pagination.js
Lines 3 (patched)
<https://reviews.apache.org/r/62607/#comment263027>

    nit - `maxPages` and `numPages` are a little confusing. Can we use the terminology similar
to Reactable - such as `itemsPerPage` and `pageButtonLimit` or similar?



ui/src/main/js/components/Pagination.js
Lines 3 (patched)
<https://reviews.apache.org/r/62607/#comment263044>

    nit - maxPages and numPages are a little confusing. Can we use the terminology similar
to Reactable - such as itemsPerPage and pageButtonLimit or similar?



ui/src/main/js/components/__tests__/Pagination-test.js
Lines 155-158 (patched)
<https://reviews.apache.org/r/62607/#comment263042>

    The order will be &laquo;, 7, 8, 9 correct?



ui/src/main/js/components/__tests__/Pagination-test.js
Lines 167-173 (patched)
<https://reviews.apache.org/r/62607/#comment263045>

    nit - re-order?


- Santhosh Kumar Shanmugham


On Sept. 27, 2017, 10:51 a.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62607/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2017, 10:51 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Replace Preact and custom testing with React + jest + Enzyme now that they are MIT licensed.
> 
> Side-effects:
> 
> * The Reactable library is not compatible with React 16.0 (the MIT licensed version we
are forced to use) and attempts to flag this started as early as June this year. But the author
seems to have abandoned the library. So I had to create my own Pagination class. I found myself
repeatedly using Reactable in places where I just wanted pagination (for lists of divs) anyway,
so maybe this is a good thing.
> * Deleted custom (and questionable) shallow renderer.
> * The download time will be reduced due to not having to download PhantomJS.
> 
> 
> Diffs
> -----
> 
>   build.gradle 460500aaeab28dcfeb29ec602d057ea4bca12378 
>   ui/karma.conf.js d42cda7d025f8c5618039888f4fb49ff388c3af1 
>   ui/package.json d6802029e0aeb9f6692ec8cc065949ce17b2ca07 
>   ui/src/__mocks__/react.js PRE-CREATION 
>   ui/src/main/js/components/Pagination.js PRE-CREATION 
>   ui/src/main/js/components/RoleList.js 32595601aae06730f537d37d95be2e746f779103 
>   ui/src/main/js/components/__tests__/Breadcrumb-test.js 18af0fe6c360f375fb0fa1694304d821041d9e16

>   ui/src/main/js/components/__tests__/Home-test.js 8e6bc09050148ea9711dbfe6f52b83c8210262be

>   ui/src/main/js/components/__tests__/Pagination-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Home-test.js 4f13f99ea24c0252b5f6cbb6980e0c96832d5120

>   ui/src/main/js/utils/ShallowRender.js 52e8bb259264e7dd47ee97cec35553acc147e818 
>   ui/src/main/js/utils/__tests__/ShallowRender-test.js d5663a7c44de795a5aa0b28d4f373871ffcc3fd2

>   ui/src/main/sass/components/_tables.scss 58f176cffcdae34de7c776e31b3f0342efa3024f 
>   ui/test-setup.js PRE-CREATION 
>   ui/webpack.config.js e7cd672df62006bd5c5b60a6ba0903e16a767d13 
> 
> 
> Diff: https://reviews.apache.org/r/62607/diff/3/
> 
> 
> Testing
> -------
> 
> $ ./gradlew ui:lint
> $ ./gradlew ui:test
> 
> I also tested the UI in my Vagrant image.
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message