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

choosing a neutral keyword for naming in the proto file #387

Open
lambdaR opened this issue Feb 24, 2022 · 3 comments
Open

choosing a neutral keyword for naming in the proto file #387

lambdaR opened this issue Feb 24, 2022 · 3 comments

Comments

@lambdaR
Copy link
Contributor

lambdaR commented Feb 24, 2022

a new case, similar to #382 accrued when running the dart generate workflow https://github.com/GWT-M3O-TEST/GWT-m3o-dart/runs/5320273604?check_suite_focus=true#step:18:16 .

the keyword 'New' in the protobuf file of the chat service

rpc New(NewRequest) returns (NewResponse);

will be converted into

/// Create a new chat room
Future<NewResponse> new(NewRequest req) async {
		Request request = Request(
			service: 'chat',
			endpoint: 'New',
			body: req.toJson(),
		);
  
		try {
			Response res = await _client.call(request);
			if (isError(res.body)) {
			  final err = Merr(res.toJson());
			  return NewResponse.Merr(body: err.b);
			}
			return NewResponseData.fromJson(res.body);
		  } catch (e) {
			throw Exception(e);
		  }
	}

the 'new' keyword, is a reserved optional keyword in dart and the compiler get confused about it.

we need to follow a standard for naming things in protobuf to avoid any conflicts (recommended), or we can handle these cases in every generator accordingly by prefixing the conflicted names with something which will lead to inconsistency between M3O clients.

@asim
Copy link
Member

asim commented Feb 24, 2022

Do we just move these things to Create following a crud standard? How many cases of New exist?

@lambdaR
Copy link
Contributor Author

lambdaR commented Feb 24, 2022

just one for now .... coming form chat service
we can use the crud standard or we could just name it ChatNew for example i.e the service name first then the verb ... or any standard that prevent these potential conflicts .

@asim
Copy link
Member

asim commented Feb 24, 2022

OK I've renamed it to Create. Should see the changes everywhere soon. It's at least commited.

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

2 participants