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

Fix the order of 'expected' and 'actual' values #278

Closed
wants to merge 1 commit into from
Closed

Fix the order of 'expected' and 'actual' values #278

wants to merge 1 commit into from

Conversation

hakantunc
Copy link

No description provided.

@rbasso
Copy link
Contributor

rbasso commented Sep 3, 2016

Thank you for the PR and sorry for the delay. I was traveling without a computer, and arrived home a few hours ago.

This is related to #263, that identified a few exercises where I stupidly confused (~=?) and (~?=).
The problem was accidentally fixed in branch hspec-fail-fast, when the exercises where rewritten to use hspec, and everything would be solved with #211, after merging it back to master.

Unfortunately, the migration to hspec is being much slower than expected, and it will probably not be finished soon. My fault!

We have some options here:

  1. Merge the PR, creating a conflict when merging hspec-fail-fail to master.
  2. Bring the new version from hspec-fail-fast to master, avoiding the conflict.
  3. Merge hspec-fail-fast to master right now and continue the migration to hspec directly in master, avoiding similar problems in the future.

I don't know much about git and GitHub, so I'm not very confident about what is the best decision here, but I'm inclined to follow the third option.

What do you think?

Anyone else has an opinion on this?

@petertseng
Copy link
Member

Can you clarify - does applying the second option mean that on master only all-your-base would be using hspec?

What disadvantages do you see with taking the third option? That students will get hspec with some exercises but hunit with others? And you wish to avoid having them see that inconsistency?

If the disadvantages are acceptable, it seems good to let students reap the benefits of the work sooner, and take the third option.

If the disadvantages are not acceptable, I would suggest to merge this PR then rebase hspec-fail-fast on master to resolve the conflict sooner rather than later.

@rbasso
Copy link
Contributor

rbasso commented Sep 3, 2016

Can you clarify - does applying the second option mean that on master only all-your-base would be using hspec?

Yes. Sounds horrible, right?

What disadvantages do you see with taking the third option? That students will get hspec with some exercises but hunit with others? And you wish to avoid having them see that inconsistency?
If the disadvantages are acceptable, it seems good to let students reap the benefits of the work sooner, and take the third option.

I regret creating the hspec-fail-fast branch, it was not really needed. My original plan was to finish everything in less than a month, so that would not be a huge problem, and we would be able to migrate all exercises at the same time. It was a bad idea, sorry!

So, maybe it's already time to:

Sorry for the trouble, @hakantunc. 🙇

@petertseng
Copy link
Member

OK, I'm all for merging hspec-fail-fast now.

Because no matter when we merge it, there will likely be students partway through the track who were seeing hunit and now suddenly they see hspec. So it's not like waiting avoids that. The only thing that might happen is: a student was partway through, used to see hunit, and then suddenly sees hspec... then finishes enough problems and sees hunit again.

If a student gets bothered by that, maybe student can help out with the conversion? =D

I haven't done any myself since I've had other matters to deal with myself as well.

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.

3 participants