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

Update the use of "Machines" in Che #13703

Closed
slemeur opened this issue Jul 4, 2019 · 15 comments
Closed

Update the use of "Machines" in Che #13703

slemeur opened this issue Jul 4, 2019 · 15 comments
Labels
kind/bug Outline of a bug - must adhere to the bug report template.

Comments

@slemeur
Copy link
Contributor

slemeur commented Jul 4, 2019

Description

Along with the issue:
#13667

Machine terminology is deprecated and should be updated.

Example:
https://github.com/eclipse/che/blob/07263f1e30089689d71b057f747a44a29283e3c4/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/hc/ServerChecker.java#L80

@slemeur slemeur added kind/bug Outline of a bug - must adhere to the bug report template. severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. target/che7GA labels Jul 4, 2019
@tsmaeder
Copy link
Contributor

tsmaeder commented Jul 5, 2019

@slemeur could you explain why you see this as a blocker? Everything works just fine and I wouldn't classify the use of "machine" as a major embarassment or usability problem.

@slemeur
Copy link
Contributor Author

slemeur commented Jul 5, 2019

We deprecated that naming long time ago, it should have been handled already. Consistency matters as users might get confused by the mismatching of this wording used in other places.

As we mentioned for #13667, we should analyze the impacts and prioritize once we have an understanding of those.

cc @skabashnyuk

@tsmaeder tsmaeder added this to the 7.1.0 milestone Jul 5, 2019
@tsmaeder
Copy link
Contributor

tsmaeder commented Jul 5, 2019

I really don't see this as a precondition for shipping 7.0 Feel free to raise a discussion about this.

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Jul 8, 2019

@slemeur if we are talking only about this concrete piece of code

String.format("Server '%s' in machine '%s' not available.", serverRef, machineName)); 

we can make it more neutral for GA. However, if we are talking about removing all referencing of machines in the internal core of the workspace orchestration engine I doubt that that is doable for GA..

@l0rd WDYT?

@slemeur
Copy link
Contributor Author

slemeur commented Jul 8, 2019

Can we get the list of all error messages like this which are exposing the term?

@l0rd
Copy link
Contributor

l0rd commented Jul 8, 2019

@skabashnyuk I am +1 to do that work extensively on all the code base targetting 7.1.0. We are too close to GA to do this work now. Anyway changing only this single error message for 7.0.0 should not be too hard neither.

@skabashnyuk
Copy link
Contributor

Anyway changing only this single error message for 7.0.0 should not be too hard neither.

Sure, we just looking for similar places to fix them all at once.

@skabashnyuk
Copy link
Contributor

@l0rd can you clarify expectation #13703 (comment). Is this #13808 something that we might/need to merge for GA. Your comment, labels and a milestone of this issue can be interpreted wrong.

@l0rd
Copy link
Contributor

l0rd commented Jul 10, 2019

@skabashnyuk it wasn't clear from your answers if the fix was simply changing the log message or doing a major refactoring.

Anyway I am +1 to target milestone 7.0.0 for the simple log message change. @slemeur is responsible for the triage today and should propose to set the milestone if he agrees with it.

@slemeur
Copy link
Contributor Author

slemeur commented Jul 10, 2019

This issue will get milestone 7.0.0.

Please @skabashnyuk create another issue for refactoring the source code. It will get postpone to a later time.

@slemeur slemeur modified the milestones: 7.1.0, 7.0.0 Jul 10, 2019
@skabashnyuk
Copy link
Contributor

Please @skabashnyuk create another issue for refactoring the source code. It will get postpone to a later time.

I would like to have a broader discussion first because the purpose and the way how we want to do that is a bit unclear to me.

@slemeur
Copy link
Contributor Author

slemeur commented Jul 16, 2019

@skabashnyuk : I think this has slip from the che 7 duties during too long. One reason is probably it was not in your backlog so you forgot and now we raise that back at a time that is not appropriated. So please, make sure the issue is in your team backlog. The discussion and the clarification (if needed) should be happening in the issue.

@slemeur slemeur reopened this Jul 16, 2019
@slemeur slemeur modified the milestones: 7.0.0, 7.x Jul 16, 2019
@skabashnyuk
Copy link
Contributor

@slemeur or @l0rd can you clarify me what issue I have to create. The scope of that task for me is unclear.

@skabashnyuk skabashnyuk modified the milestones: 7.x, 8.x Jul 19, 2019
@skabashnyuk
Copy link
Contributor

Changing to 8.x since it requires fundamental changes in API because of the notion of the machine is the base of workspace runtime object. There is no clear way how to make that without the breaking changes.

@l0rd
Copy link
Contributor

l0rd commented Jul 19, 2019

@skabashnyuk that means not working on it since 8.x will eventually be Workspace CRD based.

@skabashnyuk skabashnyuk removed this from the 8.x milestone Jul 22, 2019
@skabashnyuk skabashnyuk removed severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. target/che7GA labels Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Development

No branches or pull requests

4 participants