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 TestBugAdmin file #1446

Closed
wants to merge 2 commits into from
Closed

Conversation

z-zafirov
Copy link

@z-zafirov z-zafirov commented Mar 19, 2020

Created test_admin file to test BugAdmin view => if on admin page a user can click and be redirected to the new-bug form

@coveralls
Copy link

coveralls commented Mar 19, 2020

Coverage Status

Coverage increased (+0.06%) to 75.341% when pulling 9cea49d on z-zafirov:zdravko_wip into 81f7ace on kiwitcms:master.

Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

More changes needed:

  1. squash both commits together
  2. commit log message can be better - from the diff itself it is obvious that a new file is added and what it's name is, and the name is not TestBugAdmin. See this reference for how to write a good commit log: https://chris.beams.io/posts/git-commit/

tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 28, 2020

Codecov Report

Merging #1446 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1446      +/-   ##
==========================================
+ Coverage   76.26%   76.28%   +0.02%     
==========================================
  Files         123      123              
  Lines        4580     4580              
  Branches      547      547              
==========================================
+ Hits         3493     3494       +1     
+ Misses        912      911       -1     
  Partials      175      175              
Impacted Files Coverage Δ
tcms/bugs/admin.py 91.66% <0.00%> (+8.33%) ⬆️

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 c84bc09...db02429. Read the comment docs.

Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Also ignored from previous review:

  • squash all commits together
  • commit log message can be better - from the diff itself it is obvious that a new file is added and what it's name is, and the name is not TestBugAdmin. See this reference for how to write a good commit log: https://chris.beams.io/posts/git-commit/

tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
@zahari
Copy link

zahari commented Apr 8, 2020 via email

Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Make pylint happy & squash all commits into 1

tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
@z-zafirov z-zafirov force-pushed the zdravko_wip branch 2 times, most recently from ac4b935 to 3d0ea7d Compare April 9, 2020 19:02
Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Make pylint happy

Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Minor changes requested.

Squash commits together.

tcms/bugs/tests/test_admin.py Outdated Show resolved Hide resolved
Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Rebase & squash your commits. The PR contains unrelated changes which should not be the case.

Add tests for bugs in admin view

Remove doc string

Remove doc string
Copy link
Member

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

Drop the merge commit and the unrelated changes from the 1st commit.

@atodorov
Copy link
Member

atodorov commented May 4, 2020

Closed in favor of #1642

@atodorov atodorov closed this May 4, 2020
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.

4 participants