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

io.xseed: don't raise Userwarning if station comment end time is unset #1673

Merged
merged 1 commit into from Feb 23, 2017

Conversation

Projects
None yet
4 participants
@barsch
Member

barsch commented Feb 15, 2017

also corrected a wrong part of a test case claiming it raises on missing starttime - it raised because endtime was missing ...

fixes #1564

@krischer

This comment has been minimized.

Member

krischer commented Feb 21, 2017

Failures are unrelated and fixed in the master. Should this maybe be rebased in the maintenance_1.0.x branch?

@megies

This comment has been minimized.

Member

megies commented Feb 21, 2017

@barsch, would be good to avoid duplicate tickets.. our wiki has the curl command to make existing issues into PRs: https://github.com/obspy/obspy/wiki/Interesting-Reading-on-Git%2C-GitHub

@QuLogic

This comment has been minimized.

Member

QuLogic commented Feb 21, 2017

@krischer @megies The "fixes #1564" in the description will close the ticket when this PR is merged. There's no need to require modifying the original ticket or closing so early.

@QuLogic QuLogic added this to the 1.1.0 milestone Feb 21, 2017

@megies

This comment has been minimized.

Member

megies commented Feb 21, 2017

@krischer @megies The "fixes #1564" in the description will close the ticket when this PR is merged. There's no need to require modifying the original ticket or closing so early.

Yeah, but.. way tooooo many open tickets as it stands anyway. Adding lots of duplicated tickets is just cluttering more, so I kind of agree with @krischer in closing one of the dups. It's enough to have one ticket open and it kind of also prevents people from commenting in both of the tickets, creating more fragmentation of the issue.

@barsch

This comment has been minimized.

Member

barsch commented Feb 21, 2017

actually I don't want to force user to use some "git magic" to do PR's - I'm happy if anyone contributes even it means duplicating of tickets .... a duplicate ticket is bad, but can be closed - but losing contributers because of formal rules is even worse

@megies

This comment has been minimized.

Member

megies commented Feb 21, 2017

  • this is no additional 'git magic', we just politely keep asking people to tell us to handle the whole "turn issue into PR" thing
  • we're not discouraging anyone from contributing
  • we're just politely telling people that we are able to convert tickets into PRs for them (once they have their branch ready -- which does not mean more work for them but rather less)
  • unnecessary additional work or making our ticket tracker unclear/confusing for the core devs is a threat to the project -- contributions are nice, but at the end of the day somebody needs to polish, quality-check(, potentially rebase) and merge (just see our several-month-behind-schedule 1.0.3 release which should have been out half a year ago or the ton of tickets in our 1.1.0 milestone. it's starting to get out of hand. just my 2 cents.)
@barsch

This comment has been minimized.

Member

barsch commented Feb 22, 2017

I wasn't aware that this is not required for "normal" contributers - I read something similar in #1678 and assumed its somehow wanted from the person who opened the PR - so my bad for not reading carefully

however, for me as an contributer (with some elevated rights) its still extra work - in this case (a bugfix of a module I pretty much wrote from the scratch) it feels almost if all the other stuff around a PR is more time consuming than writing the patch itself - so yes, I feel a bit discouraged - but as I'm pretty much the only one I guess I'll have to live with that ...

@barsch barsch changed the base branch from master to maintenance_1.0.x Feb 22, 2017

@barsch barsch modified the milestones: 1.0.3, 1.1.0 Feb 22, 2017

@barsch

This comment has been minimized.

Member

barsch commented Feb 22, 2017

ok rebased to maintenance_1.0.x - not merging tickets as the other one is already closed

@megies

This comment has been minimized.

Member

megies commented Feb 22, 2017

Looks good so far, docker-deb-buildbot and Travis test fails are unrelated, see #1681. Let's wait for docker-testbot to finish..

@krischer

This comment has been minimized.

Member

krischer commented Feb 23, 2017

Failures are unrelated so I'm merging.

Regarding the duplicate issues: Maybe we should err on the side of making it easier for people to contribute as the barrier is honestly quite high already.

@krischer krischer merged commit e83b717 into maintenance_1.0.x Feb 23, 2017

2 of 5 checks passed

docker-deb-buildbot Deb packaging succeeded but tests failed
Details
docker-testbot Docker tests failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@megies megies deleted the 1564 branch Feb 23, 2017

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