search

dondi / GRNsight

Web app and service for modeling and visualizing gene regulatory networks.
http://dondi.github.io/GRNsight
BSD 3-Clause "New" or "Revised" License
17 stars 8 forks source link

Lingering style notes from #996 #1002

Closed dondi closed 6 months ago

dondi commented 1 year ago

A number of smaller style issues where noted in #996—this can be good onboarding work for new team members in the spring. Go to the closed pull requests list, find #996, then go over the comments from the last review, changing the code accordingly without incurring regressions

kdahlquist commented 6 months ago

This will be the next one that @ceciliazaragoza and @akaiap will tackle this week.

ntran18 commented 6 months ago

I also found repetitive code in scripts for generate_network.py, generate_new_network_version.py and generate_sgd_network_from_yeastract_network.py

image

ceciliazaragoza commented 6 months ago

I found a lingering /* eslint-disable max-len */ in line 1 at the top of server/dals/protein-dal.js, so removed the comment, fixed multiline string in the file, and ran lint and test with all tests passing.

ceciliazaragoza commented 6 months ago

I also found repetitive code in scripts for generate_network.py, generate_new_network_version.py and generate_sgd_network_from_yeastract_network.py

image

A'Kaia and I are working on this issue that Maika brought up, and for the second comment shouldn't the print string be print(f'Creating REGULATORS TO REGULATORS MATRIX\n')?

ceciliazaragoza commented 6 months ago

A'Kaia and I worked on the issue that Maika suggested in generate_network.py, and lint and test are all passing.

ceciliazaragoza commented 6 months ago

I also added the createMatrix method to generate_new_network_version.py and removed the redundancy in if result != False, changing to if result

dondi commented 6 months ago

@ceciliazaragoza will issue a PR to merge with beta for these changes. In addition, @kdahlquist pointed out that we need to reinstate the practice of doing QA on our pull requests in accordance with the https://github.com/dondi/GRNsight/wiki/Pull-Request-Checklist

So for this PR, @ceciliazaragoza will copy/paste the checklist on this wiki page on the pull request, and the team will work through the checklist concurrently on their respective local builds

If any checklist item regressions are found, the team will:

  1. Check to see if the regression exists in production (v6.0.8) or beta (v7.0.8)
  2. Check to see if an issue has already been reported for it (specific focus on the recent items entered by @kdahlquist)
  3. Update the PR comment thread as needed so the team knows where an issue stands
  4. Seek to fix the issue
  5. Iterate on the checklist until, as far as we know, all items are addressed

For @nchun2, her work in GRNmap sets her up as the “import/export” specialist between GRNmap and GRNsight. She can focus on items that ensure smooth file-based data flow between the two systems. @kdahlquist will write a specific issue