Issue #3 resolved

Merging upstream

Sergey Schetinin
created an issue

Hi! Thanks for the effort guys. What's the plan regarding merging these tests upstream? Can we please not do that by hg means but by creating a patch or a couple incorporating all of the updates, so that the repo history is kept relatively clean?

Comments (18)

  1. Sergey Schetinin reporter

    I didn't know about the --collapse flag, seems to be exactly what's needed. However I've just tested it w/ hg rebase --dest=573 --source=511 --collapse on the checkout of this fork and it bails w/ abort: unable to collapse, there is more than one external parent.

    Simple copy of one checkout over another followed w/ a commit works just fine (the credit to the original committers would have to go to the commit message though).

  2. Mariano Mara repo owner
    • changed status to open

    Sounds good to me. Can we wait to the weekend? I'm in the countryside with a highly limited 3G connection and I would like to add a couple of new tests during this week.

    Danny, do you feel comfortable making the checkout and commit as Sergey suggests? I'm not sure if I can pull it out without screwing everything.

    Mariano

  3. Danny Navarro

    I only included the changes for the tests. There are some modifications in ``response.py`` and ``datetime_utils.py``, those might be better in a separate commit.

  4. Sergey Schetinin reporter

    Great. However, I think we can work on the coverage some more before merging upstream.

    May I have commit access to this fork as well? I wanted to edit some of the tests and I think that those edits are best kept to this fork.

  5. Mariano Mara repo owner

    Regarding when to merge upstream, we previously discussed with Danny to try to reach 90% coverage before submitting out first merge request. With my last commit we're at 87%. What do you guys think if we keep that target?

  6. Mariano Mara repo owner

    Sergey, I intent to work until WebOb have 100% test coverage. Hope we can get there really soon :)

    Danny Xu, right now I can only log in at night (my timezone is -3) so I think our best chance to meet will be next week. I'm looking forward so I can learn more about mercurial.

  7. Sergey Schetinin reporter

    Even having 100% line coverage there's always more work to cover all the important cases and conditions. However I personally don't think having such a coverage for the sake of it is necessary (it's great when a big refactoring is planned, but I don't think WebOb needs it). Most of the time if the code itself is clear, to the point and does not depend on some kind of tricks, that's good enough. Anyway, we're quite close to 100% coverage, so why not finish it off.

  8. Sergey Schetinin reporter

    What do you guys think about merging now? Any last tests you want to get in before the merge? I'll make a couple edits before the merge anyway, but tell me once you're ready.

  9. Mariano Mara repo owner

    First things first: congrats on your now official webob maintainer role!

    Regarding your question: a few days back I decided to give webob/dec.py 100% coverage but I'm still reading the code, trying to understand what it does and what tests to write: it might take me a few more days.

    Said so, I'm ok with merging what we have up to now (we're 88% of coverage right now) so please go ahead (or let me know if I have to do something to make it happen). Regarding the credit to the committers, I really don't mind if my name is in the message, in the repo or it's not present at all. Please choose the fastest and painless way to merge.

  10. Sergey Schetinin reporter

    I think these are all of the edits I want to do for now. Please don't do things like `ok_(x==y, 'verbose verbose ...')` and multiline blocks that could be replaced with a simple assert_raises. Regarding .dec -- I'm not sure how much is it used actually.

    I'll wait with merging until tomorrow, in case Danny wants to get some tests in before the merge.

    Modesty is nice and all, but I'll credit you in the commit message and the changelog anyway.

  11. Log in to comment