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

Remove the enable-heterogeneous config option #2838

Closed

Conversation

nrgraham23
Copy link
Contributor

This commit removes the enable-heterogeneous option.
If the user attempts to use this option, configure
will print a warning message and error out.

fixes #2802

Signed-off-by: Nathaniel Graham [email protected]

@hppritcha @jsquyres

This commit removes the enable-heterogeneous option.
If the user attempts to use this option, configure
will print a warning message and error out.

Signed-off-by: Nathaniel Graham <[email protected]>
@nrgraham23 nrgraham23 added this to the v2.0.3 milestone Jan 25, 2017
@nrgraham23
Copy link
Contributor Author

Here is the fix without removing the dead code this PR creates. I can modify it to go ahead and remove that code if it is decided that is what is wanted.

Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the F2F Jan 2017 meeting, we agree that we're removing this CLI option, and ensuring to print a warning + abort configure if a user specifies it (on the rationale that we can't deliver what the user asked for, so we need to abort and let a human figure it out).

@nrgraham23
Copy link
Contributor Author

@jsquyres @hppritcha Is this ready to be merged?

@rhc54
Copy link
Contributor

rhc54 commented Jan 27, 2017

I believe that @ggouaillardet wanted to retain it and was offering to maintain it - if that is true, then we really shouldn't remove it.

@nrgraham23
Copy link
Contributor Author

He did. I assumed that Jeff and Howard had seen his comments on issue #2802 and had decided to go ahead with the removal anyways, but if this is not the case, let me know and I can close this.

@hppritcha
Copy link
Member

I think we should go ahead with this PR and when @ggouaillardet fixes existing issues with hetero he can reenable.

@rhc54
Copy link
Contributor

rhc54 commented Jan 31, 2017

@ggouaillardet We talked about this on the telecon this morning, and bottom line is that we will apply our usual policy here: if someone is willing to maintain and test this feature, then no problem leaving it in the code. This probably applies sooner to v2.1 than v2.0.3, but if you can update the support and at least post on the PR your MTT results, then we are content to leave this alive.

@jsquyres
Copy link
Member

jsquyres commented Feb 7, 2017

@ggouaillardet Do you have plans to fix the heterogeneous support?

@ggouaillardet
Copy link
Contributor

@jsquyres i made #2940 with some fixes i did not yet push into master
i will likely ask some extra pair of eyes to review them before commiting

@jsquyres
Copy link
Member

jsquyres commented Feb 8, 2017

Ok. So the intent is definitely to fix heterogeneous -- let's close this PR, then.

@jsquyres jsquyres closed this Feb 8, 2017
@nrgraham23 nrgraham23 deleted the remove_enable_heterogeneous branch February 10, 2017 17:13
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.

remove --enable-heterogenous configury option
5 participants