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

Function in templates should have parameters? #509

Closed
jankaifer opened this issue Aug 11, 2017 · 24 comments
Closed

Function in templates should have parameters? #509

jankaifer opened this issue Aug 11, 2017 · 24 comments

Comments

@jankaifer
Copy link

jankaifer commented Aug 11, 2017

I feel better if I have the parameters in template (maybe because I'm used to it from other sites). When I've been solving first problem with input I didn't knew how I'm supposed to get input. I had to look in the tester script.

@behrtam
Copy link
Contributor

behrtam commented Aug 11, 2017

But shouldn't choosing a good name for parameters be part of solving the exercises?

@jankaifer
Copy link
Author

jankaifer commented Aug 11, 2017

You can always rename it 😄
Maybe not names, just some strange placeholders, but something that tells me what my function gets as input. Something like inputString or array.

@ilya-khadykin
Copy link
Contributor

Fixed in #510

@jankaifer
Copy link
Author

jankaifer commented Aug 21, 2017

@m-a-ge well its not fixed yet because I fixed just one exercise. But others templates are still without parameters. If you find this a good idea then all exercises should be modified.
I don't know where to write it to let others know that new exercises should have parameters in template and to tell contributors that someone could upgrade other exercises as well.

@ilya-khadykin
Copy link
Contributor

@jankaifer, got it now
I'll reopen the issue then

@ilya-khadykin ilya-khadykin reopened this Aug 21, 2017
@jankaifer
Copy link
Author

Thx

@jankaifer
Copy link
Author

jankaifer commented Aug 22, 2017

Hi, could someone mark this as beginner-friendly or even first-timers-only please.
Thx

@ilya-khadykin
Copy link
Contributor

@jankaifer, I've added requested label.

@lilislilit
Copy link

#521 Mostly covers it, I hope :)

@jankaifer
Copy link
Author

jankaifer commented Sep 7, 2017

@lilislilit thanks for changing all exercises.
I'm wondering If this issue should be closed (because it's changed everywhere) or left open (as info for new exercises).

@lilislilit
Copy link

lilislilit commented Sep 7, 2017

@jankaifer As I was told in #521 comments, PR's should be separate for each exercise, so I guess, maybe add a checklist in top comment?

@jankaifer
Copy link
Author

jankaifer commented Sep 7, 2017

@m-a-ge could you please un-merge my PR #514 because as @behrtam says in #521 there should be just one exercise per PR like in #510 (as written in contributing-guide).

(Maybe we could make one exception with this rule because it will create lot of PRs. But @behrtam is the boss here 😄 .)

@ilya-khadykin
Copy link
Contributor

First of all, @lilislilit and @jankaifer thanks for working on this!
To be honest I don't want to revert #514, we can always create a new PR to improve params naming if required
And I also agree that creating PRs for each exercise can be time consuming and unnecessary

@lilislilit
Copy link

@m-a-ge I agree, I don't have that much of a preference, but it would be like...a few dozen PR's. We should come to agreement with @behrtam then.

@lilislilit
Copy link

@jankaifer I revied the list on the top, and there are far too many items there, that are already implemented. These are those that left, not that impossible(Hacktoberfest is around the corner!):

Binary Search Tree
Custom Set
Bowling
Food Chain
Bank Account
Forth
React
Compute the result for a game of Hex / Polygon
Simple Linked List(? seems like it is already done in linked-list)
Dominoes
Parallel Letter Frequency
Zipper
Pov
Spiral Matrix
Two Bucket
Complex Numbers(approved, not yet merged)
Error Handling
Markdown
Parsing a Smart Game Format string.
Write a Domain Specific Language similar to the Graphviz dot language
Go Counting
Hangman
Lens Person
Tree Building
Paasio

From these only Paasio and lenses seem like a bit pointless affairs from the python pov(non sure about the passio, though).

@ilya-khadykin
Copy link
Contributor

ilya-khadykin commented Sep 8, 2017

Hacktoberfest is around the corner!

@lilislilit, should we create a separate issue for each of mentioned exercises and label them accordingly for Hacktoberfest?

Complex Numbers(approved, not yet merged)

I've merged it, thanks

@lilislilit
Copy link

@m-a-ge I am not a maintainer here, but that should be swell. I don't think guys would mind much :) C# track are in those labels up to vazoo.

@jankaifer
Copy link
Author

jankaifer commented Sep 26, 2017

@lilislilit Could you please wrote exact names of exercises?
I found just half of your list.

@sjwarner-bp
Copy link
Contributor

I've just done this for the acronym.py template. I hope I did it ok, I am new to this! Please let me know if there is anything I could have done better.

sjwarner-bp added a commit to sjwarner/python that referenced this issue Oct 2, 2017
Added a `words` parameter to the abbreviate method to ensure clarity to future users. This change was discussed in issue exercism#509.
@ilya-khadykin
Copy link
Contributor

@jankaifer, thanks for your list. I'm creating separate issues for each of these exercises as part of Hacktoberfest.
I hope you and @lilislilit will contribute since you've committed a lot of in this.

@jankaifer
Copy link
Author

jankaifer commented Oct 4, 2017

@m-a-ge do you need help with merging PRs and closing issues?

@ilya-khadykin
Copy link
Contributor

@jankaifer it definitely take some time to go through all of them, to be honest.
Do you want to become a maintainer of the track?

@jankaifer
Copy link
Author

jankaifer commented Oct 4, 2017

@m-a-ge I would love to become a maintainer.

@N-Parsons
Copy link
Contributor

And I think we're done! Thanks for clearing the last few up, @cmccandless!

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

No branches or pull requests

6 participants