Skip to content
This repository was archived by the owner on Apr 16, 2023. It is now read-only.

Conversation

@meshy
Copy link
Owner

@meshy meshy commented Oct 7, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8464f99 on cms into accdec8 on master.

@meshy
Copy link
Owner Author

meshy commented Oct 7, 2014

nav_tree.urls need integration tests, and cms.urls needs unit tests.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Incomplete

@meshy
Copy link
Owner Author

meshy commented Oct 7, 2014

This PR too big -- will have to remove the nav_tree stuff for the moment

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d35c0d5 on cms into accdec8 on master.

Conflicts:
	conman/cms/templates/cms/index.html
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.56%) when pulling e00a42b on cms into accdec8 on master.

@kevinetienne
Copy link
Collaborator

Looks good, do you think it worth adding a test for app_url in cms/urls?

@meshy
Copy link
Owner Author

meshy commented Oct 8, 2014

Yeah, definitely. I've got a half-built test at home, so I'll commit it tonight

Thanks for having a look :)

@meshy
Copy link
Owner Author

meshy commented Oct 8, 2014

(Don't want to lose that 100% coverage!)

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8445d49 on cms into accdec8 on master.

@meshy
Copy link
Owner Author

meshy commented Oct 8, 2014

Please review!

Copy link
Collaborator

Choose a reason for hiding this comment

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

wondering if this should be an AttributeError?

Copy link
Owner Author

Choose a reason for hiding this comment

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

No.

exception AttributeError

Raised when an attribute reference (see Attribute references) or assignment fails. (When an object does not support attribute references or attribute assignments at all, TypeError is raised.)

exception ValueError

Raised when a built-in operation or function receives an argument that has the right type but an inappropriate value, and the situation is not described by a more precise exception such as IndexError.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough, I was thinking as cms_urls is an attribute it could have make sense

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sorry -- I just re-read that, and I came across really harsh, sorry! 🍰

Copy link
Collaborator

Choose a reason for hiding this comment

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

not at all, don't worry :)

Copy link
Owner Author

Choose a reason for hiding this comment

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

👍

kevinetienne pushed a commit that referenced this pull request Oct 9, 2014
@kevinetienne kevinetienne merged commit 9a7e03b into master Oct 9, 2014
@kevinetienne kevinetienne deleted the cms branch October 9, 2014 20:13
@meshy
Copy link
Owner Author

meshy commented Oct 9, 2014

Thank you! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants