Skip to content
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

Add related names to django models #2423

Closed
wants to merge 41 commits into from

Conversation

johannaengland
Copy link
Contributor

Closes #2418

Will squash the commits together after reviews

@johannaengland johannaengland self-assigned this Jun 17, 2022
@codecov
Copy link

codecov bot commented Jun 17, 2022

Codecov Report

Merging #2423 (1fd549f) into master (f4a6adc) will increase coverage by 0.06%.
The diff coverage is 39.10%.

@@            Coverage Diff             @@
##           master    #2423      +/-   ##
==========================================
+ Coverage   52.64%   52.70%   +0.06%     
==========================================
  Files         552      552              
  Lines       40167    40186      +19     
==========================================
+ Hits        21144    21180      +36     
+ Misses      19023    19006      -17     
Impacted Files Coverage Δ
python/nav/eventengine/plugins/linkstate.py 26.27% <0.00%> (ø)
python/nav/ipdevpoll/neighbor.py 50.86% <ø> (ø)
python/nav/ipdevpoll/plugins/lldp.py 30.93% <0.00%> (ø)
python/nav/ipdevpoll/plugins/statports.py 35.63% <0.00%> (ø)
python/nav/ipdevpoll/shadows/__init__.py 42.64% <0.00%> (-0.03%) ⬇️
python/nav/ipdevpoll/shadows/netbox.py 59.25% <0.00%> (ø)
python/nav/models/arnold.py 91.20% <ø> (ø)
python/nav/models/event.py 79.73% <ø> (ø)
python/nav/models/images.py 77.08% <ø> (ø)
python/nav/models/msgmaint.py 79.54% <0.00%> (ø)
... and 54 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4a6adc...1fd549f. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jun 17, 2022

Test results

     12 files       12 suites   12m 14s ⏱️
3 086 tests 2 990 ✔️   96 💤 0
8 733 runs  8 445 ✔️ 288 💤 0

Results for commit 1fd549f.

♻️ This comment has been updated with latest results.

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. What a beast. Other than my multiple inline comments, I have two requests for the future:

  1. Please break things like this into multiple PRs. It's a nightmare to review and keep track of changes underway.
  2. Please do not force-push changes after a review. Adding more commits makes it easier to see what has changed since the last review - and everything can be squashed once approved.

python/nav/models/manage.py Outdated Show resolved Hide resolved
python/nav/models/event.py Outdated Show resolved Hide resolved
python/nav/models/event.py Outdated Show resolved Hide resolved
python/nav/models/event.py Outdated Show resolved Hide resolved
python/nav/models/event.py Outdated Show resolved Hide resolved
python/nav/models/manage.py Outdated Show resolved Hide resolved
python/nav/models/manage.py Outdated Show resolved Hide resolved
python/nav/models/manage.py Outdated Show resolved Hide resolved
python/nav/models/manage.py Outdated Show resolved Hide resolved
python/nav/models/manage.py Outdated Show resolved Hide resolved
@johannaengland
Copy link
Contributor Author

The force push was because of a rebase onto master

@johannaengland
Copy link
Contributor Author

Yes, much better, but there are still loads of comments from my last review that still have not been addressed.

Should be fixed now - Github tried to hide these comments from me 😄

@sonarcloud
Copy link

sonarcloud bot commented Jul 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johannaengland
Copy link
Contributor Author

Due to the massive size of this PR I have split it up into several smaller ones.

@johannaengland johannaengland deleted the related-names branch January 9, 2023 14:28
@johannaengland johannaengland restored the related-names branch January 9, 2023 14:28
@johannaengland johannaengland deleted the related-names branch March 3, 2023 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add related names to django models
3 participants