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

Release 0.10 #238

Closed
stefankoegl opened this issue Oct 7, 2014 · 21 comments
Closed

Release 0.10 #238

stefankoegl opened this issue Oct 7, 2014 · 21 comments
Labels
Milestone

Comments

@stefankoegl
Copy link
Member

Only two weeks since 0.9, but a lot of changes have been merged already. Also the 0.10 milestone does not list any open pull requests.

Therefore I'd like to ask if there are some specific pull requests to be merged, bugs to be fixed for the release of 0.10. Otherwise I'd prepare the release in the next few days.

@DPH
Copy link
Member

DPH commented Oct 7, 2014

+1 agree
only thing I would like to fix is the error on events code when a radio station is played.

We could either use the "first attempt" at changing the data structures to cater for radio objects #231 (which prevents the error and gives data for events - however may not be the best code!) or return the data needed from events in the short term (#227) or a version of that which I have that returns the various metadata items as an xml variable. The later change is to events just 2 lines!:

134                    if value.startswith('<DIDL-Lite'):

   +                     #for debug purposes include metadata in dictionary
   +                    if log.getEffectiveLevel() == logging.DEBUG:
   +                         result['DEBUG_DIDL_'+tag] = value

Welcome comments
Cheers David

@robwebset
Copy link
Contributor

I think 0.10 is a good idea fairly soon - but do agree with David that we should really try and get the Radio Station Events fixed first - I'd favour the long term fix rather than the short term hack.

@stefankoegl stefankoegl added this to the 0.10 milestone Nov 9, 2014
@stefankoegl
Copy link
Member Author

I think we're ready for 0.10 now. Any objections? /CC @lawrenceakka, @KennethNielsen

@lawrenceakka
Copy link
Contributor

Good to go I think.

Lawrence

@robwebset
Copy link
Contributor

I think there are still several pull requests sitting in the queue - several of them fairly simple, shouldn't we review all of those first, we've had some pull requests that are more recent than those already pulled in, any reason we are not pulling in the older ones?

@stefankoegl
Copy link
Member Author

I know that there are quite a few pending pull requests, but we have to make a cut somewhere, especially if we're talking about new features (and not bug fixes).

I'd propose to fix #255 before the release (as blocks support for Python 3.2), and include the opening tickets in 0.11 (which can follow 0.10 quite soon).

@robwebset
Copy link
Contributor

I do agree that there has to be a cut-off, but surely we should be looking at pulling things in that were requested 4 weeks ago or more? (And if not - at least commenting why they shouldn't be included - i.e. they just don't do what they suggest they do)

If you look back to 12th October and earlier there are several requests from @dajobe @trollkarlen @simonalpha that have been there a while.

A cynical person may suggest that we wait until all pulls from @lawrenceakka @KennethNielsen @stefankoegl make it in, then just do the release at that point :-)

That said, I've got the pull requests I made in, so I'm not sure why I'm too bothered. Just thought I'd raise it. (I know I'd be a little fed up if I had a pull submitted that sat there not being actioned and the release just went ahead without it)

@trollkarlen
Copy link
Contributor

True that!

It doesn't even work without my fix because of my dual network interfaces.

/R
On Nov 11, 2014 3:30 PM, "robwebset" notifications@github.com wrote:

I do agree that there has to be a cut-off, but surely we should be looking
at pulling things in that were requested 4 weeks ago or more? (And if not -
at least commenting why they shouldn't be included - i.e. they just don't
do what they suggest they do)

If you look back to 12th October and earlier there are several requests
from @dajobe https://github.com/dajobe @trollkarlen
https://github.com/trollkarlen @simonalpha
https://github.com/simonalpha that have been there a while.

A cynical person may suggest that we wait until all pulls from
@lawrenceakka https://github.com/lawrenceakka @KennethNielsen
https://github.com/KennethNielsen @stefankoegl
https://github.com/stefankoegl make it in, then just do the release at
that point :-)

That said, I've got the pull requests I made in, so I'm not sure why I'm
too bothered. Just thought I'd raise it. (I know I'm be a little fed up if
I had a pull submitted that sat there not being actioned and the release
just went ahead without it)


Reply to this email directly or view it on GitHub
#238 (comment).

@KennethNielsen
Copy link
Member

@robwebset and @trollkarlen. I think you are misinterpreting disorder as (un)-preferential treatment. I agree that it might look like we are reviewing and pulling each others work first, but we don't really do that, it is more a matter of coincidence. That being said, fell free to give a hand in the review and test phase.

With regards to the specific PR's:

As far a #246 go, I would suggest not pushing to include it in this release. It is a rather big addition, and I would rather have a bit more time for review and testing, but I have asked @dajobe who is the author of it, to complain here, if he disagrees.

#242 is probably a good candidate for inclusion (seeing as it is a bugfix and has already seen some work), but it is in a part of the code that I'm not particularly familiar with @stefankoegl @lawrenceakka, what do you think.

Then there are some PR's that have not seen any activity yet.

#241 is cosmetic, so no need to rush that
#245 seems like a fix
#240 is new work

In general it is regrettable that they have not seen any activity yet, but I don't think we should rush them just to have them included in a release. We have a fairly quick release cycle, but maybe we should try and make the next one even smaller and get these changes in.

@trollkarlen
Copy link
Contributor

Don't really care for the reason, I just want my fix in so I can use the
def version. As it dont work at the moment, and soon will not either ;-)

Also always good to have short and fast integration iterations to keep
patchers from maintaining the pathes in void for ad short time as possible.

/R
On Nov 11, 2014 4:29 PM, "Kenneth Nielsen" notifications@github.com wrote:

@robwebset https://github.com/robwebset and @trollkarlen
https://github.com/trollkarlen. I think you are misinterpreting
disorder as (un)-preferential treatment. I agree that it might look like we
are reviewing and pulling each others work first, but we don't really do
that, it is more a matter of coincidence. That being said, fell free to
give a hand in the review and test phase.

With regards to the specific PR's:

As far a #246 #246 go, I would suggest
not pushing to include it in this release. It is a rather big addition, and
I would rather have a bit more time for review and testing, but I have
asked @dajobe https://github.com/dajobe who is the author of it, to
complain here, if he disagrees.

#242 #242 is probably a good candidate
for inclusion (seeing as it is a bugfix and has already seen some work),
but it is in a part of the code that I'm not particularly familiar with
@stefankoegl https://github.com/stefankoegl @lawrenceakka
https://github.com/lawrenceakka, what do you think.

Then there are some PR's that have not seen any activity yet.

#241 #241 is cosmetic, so no need to
rush that
#245 #245 seems like a fix
#240 #240 is new work

In general it is regrettable that they have not seen any activity yet, but
I don't think we should rush them just to have them included in a release.
We have a fairly quick release cycle, but maybe we should try and make the
next one even smaller and get these changes in.


Reply to this email directly or view it on GitHub
#238 (comment).

@dajobe
Copy link
Contributor

dajobe commented Nov 11, 2014

@KennethNielsen I'm ok with #246 not going in now, but I'd like it soon.

Maybe the core devs could comment on the best way for PRs to get created, reviewed & merged.

@stefankoegl
Copy link
Member Author

#258 is currently blocking the release.

We should use the time until we have a fix for this to merge some of the open fixes.

@stefankoegl
Copy link
Member Author

I've prepared the release notes in the docs (#260). Please let me know if you have any comments. I hope to release the new version within a day or so.

@stefankoegl
Copy link
Member Author

I just now noticed that since #243 the URI class is gone. Was this on purpose? Should we mention it in the docs, or maybe even add it back and deprecate it properly? /cc @lawrenceakka

@KennethNielsen
Copy link
Member

@stefankoegl As I understand it, that is intentional. That class was only used if users wanted to create their own ML structure to represent a URI so that it could be added to the queue and only if did that instead of adding the uri directly with the convenience method. It was not introduced that long ago, so I think a warning along with the warning of all the other data structures changing names should suffice.

@stefankoegl
Copy link
Member Author

OK, so the recommendation should be to replace the use of URI with play_uri(), right?

@lawrenceakka
Copy link
Contributor

@stefankoegl , @KennethNielsen is correct. The URI class appeared as part of PR #147. Its not really worth deprecating it. Perhaps we should say in the Changelog: The URI class introduced in #147 has now been removed as part of the metadata reorganisation. Use play_uri instead.

Sorry, I hadn't spotted that one!

Lawrence

@KennethNielsen
Copy link
Member

Yeah or, if they actually need an object to pass around, just uses the DIDLObject class as a replacement for the URI class.

@stefankoegl
Copy link
Member Author

I have just released SoCo 0.10!

Thanks everyone for their contributions! @simonalpha, @robwebset, @petteraas, @lawrenceakka, @KennethNielsen, @hkhanna, @gpothier, @dajobe

@trollkarlen
Copy link
Contributor

Say when and I will rebase #242

/R
On Nov 17, 2014 5:38 PM, "Stefan Kögl" notifications@github.com wrote:

Closed #238 #238.


Reply to this email directly or view it on GitHub
#238 (comment).

@KennethNielsen
Copy link
Member

\o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants