Skip to content

Conversation

@marqh
Copy link
Member

@marqh marqh commented Oct 6, 2016

required update to
#2164 (already merged :/)

@@ -0,0 +1,2 @@
- Fixed a bug where the :func:`iris.util._is_circular` would return false where
Copy link
Member

Choose a reason for hiding this comment

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

Previously we started each contribution with a *, should we keep consistency with that? I know - is the correct character, but * is also allowed, and mixing them in a single list would be a bad idea so we have to pick one.

Copy link
Member Author

Choose a reason for hiding this comment

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

i'd rather stick with '-'

as this is the first entry for this collection, perhaps it can set a precedent?

I believe that the 1.10 contributions mixed the - and * use

Copy link
Member

Choose a reason for hiding this comment

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

Fine, I'd prefer if we tried to stick to the same throughout, but depending on restructured-text backend it is worst case a warning if you mix them, so I guess it doesn't matter as much as I thought.

Copy link
Member

@bjlittle bjlittle Oct 6, 2016

Choose a reason for hiding this comment

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

@marqh LGTM ... but @ajdawson is right, after a quick scan through the last few whatsnew all points are marked with * ...

... does Sphinx render the html differently with - instead of * ... I can't tell quickly from documentation, and I can't be bother building the docs to see.

http://thomas-cokelaer.info/tutorials/sphinx/rest_syntax.html#list-and-bullets doesn't mention the use of -

@marqh could I nudge you in the direction of using * ? Then I'll merge ... I'll even update the dev notes in a separate PR 😉

Copy link
Member

Choose a reason for hiding this comment

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

It should render fine with either, but if you mix the styles you get a warning from sphinx but the output is likely correct (although in some circumstances the intent may be misinterpreted. I'd advocate using strictly 1 style only.

Copy link
Member

Choose a reason for hiding this comment

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

I vote for *

@bjlittle bjlittle self-assigned this Oct 6, 2016
@marqh
Copy link
Member Author

marqh commented Oct 6, 2016

could I nudge you in the direction of using * ? Then I'll merge ... I'll even update the dev notes in a separate PR

done

ping @bjlittle

@@ -0,0 +1,2 @@
* Fixed a bug where the :func:`iris.util._is_circular` would return false where
Copy link
Member

Choose a reason for hiding this comment

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

Now that we've dealt with character number 1, we can move onto the rest 😉 Can it be changed like this:

Fixed a bug where the :func:iris.util._is_circular would erroneously return false where when the coordinate values are decreasing.

Copy link
Member

Choose a reason for hiding this comment

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

Now is that false or False 😉

@marqh
Copy link
Member Author

marqh commented Oct 6, 2016

updated with new wording, please squish and merge if you are content

@marqh
Copy link
Member Author

marqh commented Oct 6, 2016

note: tests passed with the first commit, all further commits are just changes to the what's new wording, so you may choose not to wait for travis, if you so desire

@@ -0,0 +1,2 @@
* Fixed a bug where :func:iris.util._is_circular would erroneously return False
Copy link
Member

Choose a reason for hiding this comment

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

The markup here is no longer correct, it is missing the back-ticks ( I suspect because copy-paste from the Github comment, which turned the back-ticks into an object). You actually don't need to capitalise False in this usage unless you are referring explicitly to the Python object False...

@marqh
Copy link
Member Author

marqh commented Oct 6, 2016

The markup here is no longer correct, it is missing the back-ticks

sorry, i should have spotted that; hopefully fixed now

@bjlittle
Copy link
Member

bjlittle commented Oct 6, 2016

I'm not going to wait for travis, this is safe to merge, as the first commit passed fine on travis, so that's good enough for me.

Thanks @marqh and thanks @ajdawson

@bjlittle bjlittle merged commit 37e6850 into SciTools:master Oct 6, 2016
@QuLogic QuLogic modified the milestones: v2.0, v1.11 Oct 6, 2016
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