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

Allow tags fix for Django 2.0 #40

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

madisvain
Copy link

This needs to be checked as I'm not 100% sure how the args and kwargs are meant to be rendering.

@kedare
Copy link

kedare commented Dec 20, 2017

I think you need to make your your PR pass the tests before it can be merged, right now it's breaking compatibility with Python 2.7

@janezkranjc
Copy link
Contributor

I think the problem is that Django 2.0 gets installed on python 2.7? am I reading the travis results correctly? Django 2.0 is not supported on python 2.7

@janezkranjc
Copy link
Contributor

I believe your tests are broken for python 2.7, it has nothing to do with this PR.

@janezkranjc janezkranjc mentioned this pull request Jan 3, 2018
@janezkranjc
Copy link
Contributor

@jezdez can you please retrigger the tests and consider merging this pull request, it would be greatly appreciated.

Copy link
Member

@jezdez jezdez left a comment

Choose a reason for hiding this comment

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

@madisvain Please rebase on top of master and push to the branch again since the python 2.7 related issue is fixed now.

.gitignore Outdated
@@ -28,3 +28,5 @@ cover/
.cache/
htmlcov/
coverage.xml
env/
.vscode/
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the change to the .gitignore file since they are folder local do your computer. You can easily add it to your global gitignore instead.

@jezdez
Copy link
Member

jezdez commented Jan 5, 2018

@janezkranjc See my comment about rebasing on top of master, rerunning the tests won't be enough to fix it without the changes in master.

@codecov
Copy link

codecov bot commented Jan 10, 2018

Codecov Report

Merging #40 into master will increase coverage by 0.16%.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   72.58%   72.74%   +0.16%     
==========================================
  Files           9        9              
  Lines         434      433       -1     
  Branches       54       54              
==========================================
  Hits          315      315              
+ Misses        114      113       -1     
  Partials        5        5
Impacted Files Coverage Δ
django_celery_monitor/admin.py 53.71% <16.66%> (ø) ⬆️
django_celery_monitor/utils.py 66.12% <33.33%> (+1.04%) ⬆️

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 6ec1350...9835972. Read the comment docs.

@madisvain
Copy link
Author

@jezdez should be good to go now. sorry for the late response.

@pieterdd
Copy link

If #60 gets approved and merged, this issue can probably be closed.

@eigenmannmartin
Copy link
Member

@jezdez what is your timeline about this PR?

keep that great work up!

abusquets added a commit to Microdisseny/django-celery-monitor that referenced this pull request Mar 6, 2019
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.

6 participants