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

rna-transcription: make a lot of noise about deleting @Ignores #426

Closed
jtigger opened this issue Apr 17, 2017 · 19 comments
Closed

rna-transcription: make a lot of noise about deleting @Ignores #426

jtigger opened this issue Apr 17, 2017 · 19 comments

Comments

@jtigger
Copy link
Contributor

jtigger commented Apr 17, 2017

I have not encountered at least two solutions that pass the first test case swimmingly, but clearly wouldn't pass the whole suite:

This should be very rare.

I'm taking this as a sign we're not making enough noise about deleting @Ignores.

Please update rna-transcription so that it is hard to miss the fact that once the first test case succeeds the practitioner is reminded to delete the next @Ignore.

Options (include as many as you think would be sufficient):

  • include a HINT.md
  • put comments in the test suite.
  • add some console output that prints after the first test succeeds.
  • other?
@stkent
Copy link
Contributor

stkent commented Apr 17, 2017

We could adopt the xcsharp-style hints in @Ignore annotations:

@ignore takes an optional default parameter if you want to record why a test is being ignored

I'm not sure that's enough by itself, but it can't hurt!

@ajayboseac
Copy link
Contributor

ajayboseac commented Apr 18, 2017

@jtigger what is the use of the@Ignore tags ? Why do we want to ignore the test cases ?

@jtigger
Copy link
Contributor Author

jtigger commented Apr 19, 2017

Great question @ajayboseac. The historical discussion / policy is #101. tl;dr — to make it crystal clear what failing test should be addressed at any given moment.

@stkent
Copy link
Contributor

stkent commented Apr 21, 2017

Thanks to @FridaTveit, we've now added hints to the ignore statements in rna-transcription to prompt users to un-ignore noninitial tests. What else should we, or do we need to, do? My current thoughts are as follows:

  • add the same hints to every test suite ignore statement, which I believe is how e.g. xcsharp operates;
  • add explicit guidance via the HINTS.md file to the first exercise that contains ignored tests. This might be best placed in the new exercise that's going to replace the current (overcomplicated) hello-world.
  • capture the decisions we make in a policy, somewhere. (ref. Centralize policies #425)

@FridaTveit
Copy link
Contributor

I think that sounds like a good plan :)

@jtigger
Copy link
Contributor Author

jtigger commented Apr 23, 2017

Perfect!

@FridaTveit
Copy link
Contributor

For adding the Ignore hints to every test suite, should we make "good first patch" issues for that or should we just do it?

@jtigger
Copy link
Contributor Author

jtigger commented Apr 25, 2017

At the risk of sounding like a pedantic bureaucrat 😁 ... we currently have 11 "good first patch" issues and the policy says keep logging until we get to 20.

That said, this is your hobby-time... if you'd get a kick out of making sure this bit is completed, I think you should do whatever you'd like. :)

One thing to keep in mind is that, at some point, you're going to want to move on from being an Exercism maintainer1... one of the things I hope we can start doing is bringing in a few more volunteers to level-up as maintainers: this would make a decision to step away as a maintainer a guilt-free one and help keep the track going healthy. This is one of the main motivations for having a healthy "good first patch" backlog.

/long-winded-explanation


1 don't get me wrong: you're welcome here for as long as you want to stay!!! This notion of preparing for one's eventual departure by helping mentor someone into their position is our hope in making Exercism sustainable.

@FridaTveit
Copy link
Contributor

Okay, sounds good :) Yes, I agree that having many "good first patch" issues is important. That definitely makes it easier to contribute, and I've missed that on other projects I've tried to contibute to :)

@jtigger
Copy link
Contributor Author

jtigger commented Apr 27, 2017

Thanks for doing this legwork, @FridaTveit! It's very neighborly! :)

@morrme
Copy link
Contributor

morrme commented May 3, 2017

I am a new(ish) contributor. Am I limited to just fixing one of the issues?

@FridaTveit
Copy link
Contributor

No, you can fix as many as you like, just like you have been doing :) Thanks! :)

@morrme
Copy link
Contributor

morrme commented May 4, 2017

@FridaTveit I moved over to the ones that were not marked as "good first patch" so that I would not take too many of them from other new contributors. I see that you are working on that list as well. Would you like to divide it somehow?

@FridaTveit
Copy link
Contributor

@morrme That's fine :) I have a few in review but apart from that you can do whichever ones you want :)

@FridaTveit
Copy link
Contributor

Thank you @morrme, @mraediaz and @Smarticles101 for doing so many of these issues! :) 👍

@mraediaz
Copy link
Contributor

mraediaz commented May 7, 2017

@FridaTveit My pleasure!

@FridaTveit
Copy link
Contributor

@exercism/java do you think we've done everything we need for this now?

@Smarticles101
Copy link
Member

I have an idea. This should probably be discussed somewhere else, but what if the user was somehow warned about this by the cli? I think it would be harder to miss that way.

@stkent
Copy link
Contributor

stkent commented Jun 14, 2017

I think we're good to close out this issue.

@Smarticles101 Regarding CLI warnings; since the CLI is shared across tracks, I think it would be difficult to build in a warning mechanism that accommodates all the different test suites... if you think it's worth pursuing, I'd ask in https://github.com/exercism/cli to gauge feasibility!

@stkent stkent closed this as completed Jun 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants