> On Dec. 17, 2020, 8:27 p.m., Madhan Neethiraj wrote: > > intg/src/main/python/apache_atlas/model/instance.py > > Line 70 (original), 74 (patched) > > <https://reviews.apache.org/r/73097/diff/1/?file=2243714#file2243714line74> > > > > Many coding standards suggest line length of 80, but often adhering to > > this will result in poor readability - like in this case. > > > > I believe these guidelines were applicable for long gone era when folks > > printed code on paper, and/or the monitors (green-text?) were too narrow to > > render long lines. > > > > Wide monitors, powerful IDEs and lack of the need to print code on > > paper should enable removing this unnecessary line length limit, and > > significantly improve code readability. > > > > I strongly suggest to revert all such changes. > > Verdan Mahmood wrote: > I agree on this. as the only reason to specify the line length is to > enforce the consistency. And if we agree on one line length, I am perfectly > fine with this. BUT in any case, we should specify the maximum line length. > For many new projects, we are using `120` as the max line length. > > Madhan Neethiraj wrote: > 120 is much better than 80. However, the key is readablity than complying > to a number.
I think another angle in this discussion would be to look at Python client from community and developers perspective. PEP is a de-facto standard for Python formatting and if we want encourage Pythonistas to develop/extend it then we should consider how they work. While developing Python code in JVM-native IDE (like Eclipse or IntelliJ) won't raise any flags, these minor things will be a pain points for any IDE designed to work with Python (like Pycharm) - any kind of deviation from standard looks messy while developing. And if Pythonista is reviewing the code, then he/she will have much easier job looking at a familiar syntax. Why not make this part of Atlas familiar to them ? If full set of best practices/tools around Python development (like linting, static type validation) was to be enabled (and imo it should) there is no way around it really. I don't mean to start off yet another argument like 'spaces vs tabs' but having experience with different Open Source projects enabling clients for multiple languages I always appreciated when every client is designed and aligned with standards applicable to it and familiar to native users. In the end, every contribution should be for a good of end users, which might come from differnt backgrounds. Having said that, I think fixing code anf reformatting it should be two separate PRs so one doesn't block another. - Mariusz ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73097/#review222360 ----------------------------------------------------------- On Dec. 17, 2020, 1:45 p.m., Verdan Mahmood wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73097/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2020, 1:45 p.m.) > > > Review request for atlas, Madhan Neethiraj, Pinal Shah, and Sarath > Subramanian. > > > Bugs: ATLAS-4086 > https://issues.apache.org/jira/browse/ATLAS-4086 > > > Repository: atlas > > > Description > ------- > > Python Client Fixes > > > Diffs > ----- > > intg/src/main/python/apache_atlas/client/discovery.py > 5bead22e776c00988e7b19b0e6e81e8a3d3175df > intg/src/main/python/apache_atlas/model/discovery.py > 61a992ce7a86f3f4c75792cfe66d87dac8ca991c > intg/src/main/python/apache_atlas/model/instance.py > 89dbe350587d66442e599598908179c6ac8334d5 > > > Diff: https://reviews.apache.org/r/73097/diff/1/ > > > Testing > ------- > > - This fixes the indentation according to pep8. > - Fixes the basic search using post method, it was broken. > - Getting Bulk entities using GUIDs was broken, it also fix that. > > > Thanks, > > Verdan Mahmood > >
