nephila / djangocms-page-sitemap

django CMS page extension to handle sitemap customization

BSD 3-Clause "New" or "Revised" License

9 stars 21 forks source link

Fix deprecated 'edit_mode' attribute (compatible) #31

Closed jeroenpeters1986 closed 5 years ago

jeroenpeters1986 commented 5 years ago

Fix deprecated 'edit_mode' attribute which was removed as of DjangCMS 3.6 It's now backwards compatible up to DjangoCMS 3.4

This was reported in issue #30

coveralls commented 5 years ago

Coverage decreased (-1.3%) to 98.71% when pulling 7839a414b7fb003c2b08aca02ece185f31909bf1 on jeroenpeters1986:hotfix/issue-30 into 691ad709887386523104291e4a5b74b4c95851cd on nephila:develop.

coveralls commented 5 years ago

Coverage decreased (-1.3%) to 98.71% when pulling 7f620ceca36d04bb73b7897da9a20de126b92b7f on jeroenpeters1986:hotfix/issue-30 into 995ab2f1f8fa962d2bbe588d542c3ccb835aa351 on nephila:develop.

jeroenpeters1986 commented 5 years ago

@yakky This plugin is not compatible with DjangoCMS 3.6 anymore. I read that you were trying to keep up with maintainer duties, so I tried to make it as easy as possible for you to make it compatible.

This PR works (on my machine), but Travis seems to fail. I'm not quite sure if that's because of this commit. Also, the test coverage dropped because of the try/except.

jeroenpeters1986 commented 5 years ago

I changed the base branch to master, because this is a hotfix.

yakky commented 5 years ago

@jeroenpeters1986 yes, I'm trying to adress all my backlog of github notifications, real life knocked on the door recently :) I'm addressing test failures in #32 , could you please rebase this against develop once I merged it (just waiting for travis to complete its duties)

yakky commented 5 years ago

(I re-targeted it myself, I should be able to rebase on #32 after merge)

codecov[bot] commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@691ad70). Click here to learn what that means. The diff coverage is 50%.

@@            Coverage Diff            @@
##             develop     #31   +/-   ##
=========================================
  Coverage           ?   98.7%           
=========================================
  Files              ?       8           
  Lines              ?     154           
  Branches           ?      15           
=========================================
  Hits               ?     152           
  Misses             ?       2           
  Partials           ?       0

Impacted Files	Coverage Δ
djangocms_page_sitemap/cms_toolbars.py	`94.73% <50%> (ø)`

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 691ad70...7839a41. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #31 into develop will decrease coverage by 1.27%. The diff coverage is 50%.

@@             Coverage Diff             @@
##           develop      #31      +/-   ##
===========================================
- Coverage      100%   98.72%   -1.28%     
===========================================
  Files            8        8              
  Lines          154      157       +3     
  Branches        15       15              
===========================================
+ Hits           154      155       +1     
- Misses           0        2       +2

Impacted Files	Coverage Δ
djangocms_page_sitemap/cms_toolbars.py	`94.73% <50%> (-5.27%)`	:arrow_down:

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 691ad70...7839a41. Read the comment docs.

yakky commented 5 years ago

@jeroenpeters1986 I will merge this in #33 which will include django 2.0+ support Expect a release within the start of next week (I just need time to get all the build through travis)

Thanks a lot for your contribution!

jeroenpeters1986 commented 5 years ago

@jeroenpeters1986 I will merge this in #32 which will include django 2.0+ support Expect a release within the start of next week (I just need time to get all the build through travis)

Thanks a lot for your contribution!

Hey @yakky you are welcome and much more, thank yóu! Please take your time. it's really good to hear that it will also include Django 2.0 support (I'm at 1.11 myself, but will want to upgrade soon).