New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coala_html.py: Create python package from angular #46

Merged
merged 11 commits into from May 27, 2016

Conversation

7 participants
@tushar-rishav
Member

tushar-rishav commented May 15, 2016

This commit wraps the angular app within a standalone
Python package. Functionalities implemented with present
version are:

  1. Create necessary data files like files.json, coala.json
    etc. and launch the server.
  2. User can run coala and has an option to either launch the
    server or just update the data files.
  3. Static dependencies are being fetched from bower. If
    bower isn't installed then user gets a info to install bower
    first and then try again.
  4. files.json has been implemented using data obtained from
    file_dict which coala returns post analysis.

Fixes #31,
#43

CC @Uran198

@tushar-rishav

This comment has been minimized.

Show comment
Hide comment
@tushar-rishav

tushar-rishav May 16, 2016

Member

@Uran198 @AbdealiJK @sils1297 @Makman2 @SanketDG @abhsag24 Functionalities are done 👍
Ready for review. :D

Member

tushar-rishav commented May 16, 2016

@Uran198 @AbdealiJK @sils1297 @Makman2 @SanketDG @abhsag24 Functionalities are done 👍
Ready for review. :D

@tushar-rishav

This comment has been minimized.

Show comment
Hide comment
@tushar-rishav

tushar-rishav May 16, 2016

Member

Suggested changes are:

  1. Add bullets etc to show whitespace.
  2. Add necessary margin.
  3. Maybe a little icon on the right of the list that shows that here were results yielded.
  4. The file hierearchy is not sorted, makes navigation cumbersome
  5. The google code prettify library has some method to add inline comments.
    Or at least it adds class names with the line number. So you need to write angular code to add a div below that

@sils1297 These are the changes that I need to handle separately. We can go ahead and merge the current PR. I will create separate issues and work on it. 👍

Member

tushar-rishav commented May 16, 2016

Suggested changes are:

  1. Add bullets etc to show whitespace.
  2. Add necessary margin.
  3. Maybe a little icon on the right of the list that shows that here were results yielded.
  4. The file hierearchy is not sorted, makes navigation cumbersome
  5. The google code prettify library has some method to add inline comments.
    Or at least it adds class names with the line number. So you need to write angular code to add a div below that

@sils1297 These are the changes that I need to handle separately. We can go ahead and merge the current PR. I will create separate issues and work on it. 👍

@AbdealiJK

This comment has been minimized.

Show comment
Hide comment
@AbdealiJK

AbdealiJK May 16, 2016

Contributor

Code coverage is decreasing, tests needed for new Angular and Python code

Contributor

AbdealiJK commented May 16, 2016

Code coverage is decreasing, tests needed for new Angular and Python code

@tushar-rishav

This comment has been minimized.

Show comment
Hide comment
@tushar-rishav

tushar-rishav May 17, 2016

Member

Any pep8 checks oncoalahtml/_coalahtml/tests/test_projects/simple/*.py should be ignored as such files have been deleted anyway. 😔

Member

tushar-rishav commented May 17, 2016

Any pep8 checks oncoalahtml/_coalahtml/tests/test_projects/simple/*.py should be ignored as such files have been deleted anyway. 😔

@sils

This comment has been minimized.

Show comment
Hide comment
@sils

sils May 17, 2016

Member

make an ignore in the coafile?

Sincerely,

Lasse Schuirmann

lasse@schuirmann.net
http://coala-analyzer.org/ - http://viperdev.io/ - http://gitmate.com/

On 17 May 2016 at 13:29, Tushar notifications@github.com wrote:

Any pep8 checks oncoalahtml/_coalahtml/tests/test_projects/simple/*.py
should be ignored as such files have been deleted anyway. [image:
😔]


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#46 (comment)

Member

sils commented May 17, 2016

make an ignore in the coafile?

Sincerely,

Lasse Schuirmann

lasse@schuirmann.net
http://coala-analyzer.org/ - http://viperdev.io/ - http://gitmate.com/

On 17 May 2016 at 13:29, Tushar notifications@github.com wrote:

Any pep8 checks oncoalahtml/_coalahtml/tests/test_projects/simple/*.py
should be ignored as such files have been deleted anyway. [image:
😔]


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#46 (comment)

@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198

Uran198 May 17, 2016

Member

Does every single of your commits fixes #31?

Member

Uran198 commented May 17, 2016

Does every single of your commits fixes #31?

Show outdated Hide outdated .travis.yml Outdated
@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198

Uran198 May 17, 2016

Member

In 6a64180 you're deleting a bunch of files and adding one empty, but the commit message says you're restructuring something.

Member

Uran198 commented May 17, 2016

In 6a64180 you're deleting a bunch of files and adding one empty, but the commit message says you're restructuring something.

Show outdated Hide outdated coalahtml/helper.py Outdated
Show outdated Hide outdated coalahtml/tree.py Outdated
Show outdated Hide outdated coalahtml/tree.py Outdated
@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198

Uran198 May 17, 2016

Member

368a0ef and 2a57486 probably should not be separate, also you should add tests for this in the same commit. Also you should add tests for all of your python code and try to make coverage 100% for files you're adding or editing.

Member

Uran198 commented May 17, 2016

368a0ef and 2a57486 probably should not be separate, also you should add tests for this in the same commit. Also you should add tests for all of your python code and try to make coverage 100% for files you're adding or editing.

@AbdealiJK

This comment has been minimized.

Show comment
Hide comment
@AbdealiJK

AbdealiJK May 17, 2016

Contributor

@sils1297 Gitmate says:

review/gitmate/pr — This PR is not ready yet :/ (0 new problems)

0 new problems doesn't seem right

Contributor

AbdealiJK commented May 17, 2016

@sils1297 Gitmate says:

review/gitmate/pr — This PR is not ready yet :/ (0 new problems)

0 new problems doesn't seem right

@Uran198 Uran198 added the process/wip label May 18, 2016

Show outdated Hide outdated coalahtml/tree.py Outdated
Show outdated Hide outdated coalahtml/coala_html.py Outdated
Show outdated Hide outdated coalahtml/coala_html.py Outdated
Show outdated Hide outdated tests/coalaHelperTest.py Outdated
def test_get_args(self):
arg = get_args().parse_args()
self.assertFalse(arg.noupdate)

This comment has been minimized.

@Uran198

Uran198 May 27, 2016

Member

Not very helpful test.

@Uran198

Uran198 May 27, 2016

Member

Not very helpful test.

This comment has been minimized.

@tushar-rishav

tushar-rishav May 27, 2016

Member

Why not? It tests if args are being available correctly or not.

On Fri, May 27, 2016 at 2:35 PM, Attila notifications@github.com wrote:

In tests/coalaHelperTest.py
#46 (comment)
:

  •    s_dir = create_dir(os.path.abspath('S99'))
    
  •    d_dir = create_dir(os.path.abspath('D99'))
    
  •    s_test_file = os.path.join(s_dir, 'test.json')
    
  •    d_test_file = os.path.join(d_dir, 'test.json')
    
  •    with open(s_test_file, 'w') as sfp:
    
  •        json.dump({'key': 'value'}, sfp, separators=(',', ': '))
    
  •    copy_files(s_dir, d_dir)
    
  •    self.assertTrue(os.path.exists(d_test_file))
    
  •    shutil.rmtree(s_dir)
    
  •    shutil.rmtree(d_dir)
    
  • def test_get_args(self):
  •    arg = get_args().parse_args()
    
  •    self.assertFalse(arg.noupdate)
    

Not very helpful test.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/coala-analyzer/coala-html/pull/46/files/45b9d8e02c8e3c164f75093a3f89146b39bb2e61..f1d171ab1b617f884f1e0d2e8afaebd9aaca398a#r64877195,
or mute the thread
https://github.com/notifications/unsubscribe/AHDgOVaUvJor0C0KZo9_tNTit2XAMiFJks5qFrPmgaJpZM4Ie05w
.

@tushar-rishav

tushar-rishav May 27, 2016

Member

Why not? It tests if args are being available correctly or not.

On Fri, May 27, 2016 at 2:35 PM, Attila notifications@github.com wrote:

In tests/coalaHelperTest.py
#46 (comment)
:

  •    s_dir = create_dir(os.path.abspath('S99'))
    
  •    d_dir = create_dir(os.path.abspath('D99'))
    
  •    s_test_file = os.path.join(s_dir, 'test.json')
    
  •    d_test_file = os.path.join(d_dir, 'test.json')
    
  •    with open(s_test_file, 'w') as sfp:
    
  •        json.dump({'key': 'value'}, sfp, separators=(',', ': '))
    
  •    copy_files(s_dir, d_dir)
    
  •    self.assertTrue(os.path.exists(d_test_file))
    
  •    shutil.rmtree(s_dir)
    
  •    shutil.rmtree(d_dir)
    
  • def test_get_args(self):
  •    arg = get_args().parse_args()
    
  •    self.assertFalse(arg.noupdate)
    

Not very helpful test.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/coala-analyzer/coala-html/pull/46/files/45b9d8e02c8e3c164f75093a3f89146b39bb2e61..f1d171ab1b617f884f1e0d2e8afaebd9aaca398a#r64877195,
or mute the thread
https://github.com/notifications/unsubscribe/AHDgOVaUvJor0C0KZo9_tNTit2XAMiFJks5qFrPmgaJpZM4Ie05w
.

This comment has been minimized.

@Uran198

Uran198 May 27, 2016

Member

Test all the args at least. If I'll delete anyone except noupdate, tests wouldn't fail, but they should fail if I break something.

@Uran198

Uran198 May 27, 2016

Member

Test all the args at least. If I'll delete anyone except noupdate, tests wouldn't fail, but they should fail if I break something.

def test_copy_files(self):
s_dir = create_dir(os.path.abspath('S99'))
d_dir = create_dir(os.path.abspath('D99'))

This comment has been minimized.

@Uran198

Uran198 May 27, 2016

Member

Can you use tempfile for this?

@Uran198

Uran198 May 27, 2016

Member

Can you use tempfile for this?

This comment has been minimized.

@tushar-rishav

tushar-rishav May 27, 2016

Member

I had same thought at first, but I already have a method that does the same
thing. What's wrong?

On Fri, May 27, 2016 at 2:36 PM, Attila notifications@github.com wrote:

In tests/coalaHelperTest.py
#46 (comment)
:

  •    return_file_path = get_file(self.file_name, self.test_dir_path)
    
  •    self.assertEqual(test_file_path, return_file_path)
    
  • def test_parse_file_dict(self):
  •    result_file_data = parse_file_dict(self.test_file_dict)
    
  •    self.assertEqual(result_file_data, self.test_file_data)
    
  • def test_create_dir(self):
  •    self.assertEqual(create_dir(self.test_dir_path),
    
  •                     self.test_dir_path)
    
  •    os.rmdir(self.test_dir_path)
    
  • def test_copy_files(self):
  •    s_dir = create_dir(os.path.abspath('S99'))
    
  •    d_dir = create_dir(os.path.abspath('D99'))
    

Can you use tempfile for this?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/coala-analyzer/coala-html/pull/46/files/45b9d8e02c8e3c164f75093a3f89146b39bb2e61..f1d171ab1b617f884f1e0d2e8afaebd9aaca398a#r64877363,
or mute the thread
https://github.com/notifications/unsubscribe/AHDgOTCvoiUJG77CXSp-b2-8lDAqYghmks5qFrQzgaJpZM4Ie05w
.

@tushar-rishav

tushar-rishav May 27, 2016

Member

I had same thought at first, but I already have a method that does the same
thing. What's wrong?

On Fri, May 27, 2016 at 2:36 PM, Attila notifications@github.com wrote:

In tests/coalaHelperTest.py
#46 (comment)
:

  •    return_file_path = get_file(self.file_name, self.test_dir_path)
    
  •    self.assertEqual(test_file_path, return_file_path)
    
  • def test_parse_file_dict(self):
  •    result_file_data = parse_file_dict(self.test_file_dict)
    
  •    self.assertEqual(result_file_data, self.test_file_data)
    
  • def test_create_dir(self):
  •    self.assertEqual(create_dir(self.test_dir_path),
    
  •                     self.test_dir_path)
    
  •    os.rmdir(self.test_dir_path)
    
  • def test_copy_files(self):
  •    s_dir = create_dir(os.path.abspath('S99'))
    
  •    d_dir = create_dir(os.path.abspath('D99'))
    

Can you use tempfile for this?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/coala-analyzer/coala-html/pull/46/files/45b9d8e02c8e3c164f75093a3f89146b39bb2e61..f1d171ab1b617f884f1e0d2e8afaebd9aaca398a#r64877363,
or mute the thread
https://github.com/notifications/unsubscribe/AHDgOTCvoiUJG77CXSp-b2-8lDAqYghmks5qFrQzgaJpZM4Ie05w
.

This comment has been minimized.

@Uran198

Uran198 May 27, 2016

Member

With contextmanager you won't have to delete it manually, so it'll be easier to go through or modify.

@Uran198

Uran198 May 27, 2016

Member

With contextmanager you won't have to delete it manually, so it'll be easier to go through or modify.

Show outdated Hide outdated tests/coalaHelperTest.py Outdated
Show outdated Hide outdated coalahtml/tree.py Outdated
@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198
Member

Uran198 commented May 27, 2016

@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198

Uran198 May 27, 2016

Member

f03ab8e I still don't like commit message, maybe just files.js: Create file hierarchy

Member

Uran198 commented May 27, 2016

f03ab8e I still don't like commit message, maybe just files.js: Create file hierarchy

@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198
Member

Uran198 commented May 27, 2016

files.js: Create file hirerachy
This commit:
1.Replaces `tests/test_projects/*` with
`data` directory.
2.Create file hierarchy using `data/` files

Refs #31
@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198

Uran198 May 27, 2016

Member

f03ab8e unack.

Member

Uran198 commented May 27, 2016

f03ab8e unack.

@Uran198

This comment has been minimized.

Show comment
Hide comment
@Uran198
Member

Uran198 commented May 27, 2016

@tushar-rishav

This comment has been minimized.

Show comment
Hide comment
@tushar-rishav
Member

tushar-rishav commented May 27, 2016

@rultor merge

@rultor

This comment has been minimized.

Show comment
Hide comment
@rultor

rultor May 27, 2016

@rultor merge

@tushar-rishav OK, I'll try to merge now. You can check the progress of the merge here

rultor commented May 27, 2016

@rultor merge

@tushar-rishav OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 5790787 into master May 27, 2016

5 of 7 checks passed

codecov/patch 0% of diff hit (target 48.54%)
Details
codecov/project 45.04% (-3.50%) compared to 69f1027
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
review/gitmate/commit No issues with this one - go ahead! :)
Details
review/gitmate/manual This commit was acknowledged.
Details
review/gitmate/pr All is well! :) (0 problems solved)
Details
@rultor

This comment has been minimized.

Show comment
Hide comment
@rultor

rultor May 27, 2016

@rultor merge

@tushar-rishav Done! FYI, the full log is here (took me 1min)

rultor commented May 27, 2016

@rultor merge

@tushar-rishav Done! FYI, the full log is here (took me 1min)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment