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

Renamed --ponythreads to ponymaxthreads, fixes #3318 #3334

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Renamed --ponythreads to ponymaxthreads, fixes #3318 #3334

merged 1 commit into from
Oct 9, 2019

Conversation

alexsnaps
Copy link
Contributor

@alexsnaps alexsnaps commented Oct 8, 2019

I think fixes #3318

But this also means specifying --ponythreads would be silently ignored:

./helloworld --ponythreads=1              
Hello, world.
$ ./helloworld --ponythreads=0   
Hello, world.
$ ./helloworld --ponymaxthreads=1
Hello, world.
$ ./helloworld --ponymaxthreads=0
--ponymaxthreads can't be less than 1

Should we alias the two? Forbid --ponythreads as an option? Maybe make sure the --pony* "namespace" only contains "known" options? … thinking out loud

@alexsnaps
Copy link
Contributor Author

If we go for the "namespace concept" should it be more "explicit", e.g. XML like --pony:maxthreads or something? (still thinking out loud)

@SeanTAllen
Copy link
Member

@alexsnaps re: namespacing. that needs an RFC.

@SeanTAllen
Copy link
Member

--ponythreads shouldn't be aliased. it dies with this change.

@alexsnaps
Copy link
Contributor Author

alexsnaps commented Oct 9, 2019

I think an RFC might make sense… But could also be a drastic approach to simply "aliasing" or slowly deprecating; e.g. alias first, then print something to stderr, err out and finally go back to this behavior… I'll think it over and might indeed open an RFC. Will refresh my mind again on how other runtime do it…

@SeanTAllen
Copy link
Member

We are pre-1.0. Deprecating isn't needed.

@alexsnaps
Copy link
Contributor Author

--ponythreads shouldn't be aliased. it dies with this change.

If we're all good with this… sure. Don't get me wrong, I'm not against merging it this way (if this PR is all good with reviewers), just thought I'd explicitly mention it

We are pre-1.0. Deprecating isn't needed.

Makes sense!

@alexsnaps
Copy link
Contributor Author

In any case, this will require https://github.com/ponylang/ponylang-website changes again too.

@SeanTAllen
Copy link
Member

@alexsnaps yup. website will need to be updated. good catch. i had forgotten about that, because i am the great forgetter today.

@SeanTAllen
Copy link
Member

Thanks @alexsnaps. Can you add release notes to the release issue?

@SeanTAllen SeanTAllen merged commit a71bbe6 into ponylang:master Oct 9, 2019
@alexsnaps
Copy link
Contributor Author

Can you add release notes to the release issue?

Adding them now… will also open a PR against the website repo. Probably only later today (🤞) tho.

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.

Rename --ponythreads to --ponymaxthreads
2 participants