Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(844)

Issue 93743006: Use package names as import prefixes when generating code (Closed)

Created:
7 years ago by Søren Gjesse
Modified:
6 years, 11 months ago
CC:
reviews_dartlang.org
Base URL:
https://github.com/dart-lang/dart-protoc-plugin.git@master
Visibility:
Public.

Description

Use package names as import prefixes when generating code The generated code does not include the package name in the generated top-level classes. If several packages have the same class this causes conflicts. This change will add a prefix for each import in the generated file to solve this. The prefix used is the package name. R=sigmund@google.com BUG=https://github.com/dart-lang/dart-protobuf/issues/12 Committed: https://github.com/dart-lang/protoc-plugin/commit/24f06a2

Patch Set 1 #

Total comments: 9

Patch Set 2 : Addressed review commetns #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -37 lines) Patch
M lib/code_generator.dart View 2 chunks +4 lines, -2 lines 0 comments Download
M lib/enum_generator.dart View 3 chunks +5 lines, -1 line 0 comments Download
M lib/extension_generator.dart View 4 chunks +11 lines, -13 lines 0 comments Download
M lib/file_generator.dart View 1 3 chunks +16 lines, -1 line 0 comments Download
M lib/message_generator.dart View 5 chunks +15 lines, -9 lines 0 comments Download
M lib/protobuf_field.dart View 6 chunks +44 lines, -7 lines 0 comments Download
M test/message_generator_test.dart View 3 chunks +5 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Søren Gjesse
7 years ago (2013-12-20 14:12:34 UTC) #1
Siggi Cherem (dart-lang)
lgtm, minor comments below. https://chromiumcodereview.appspot.com/93743006/diff/1/lib/file_generator.dart File lib/file_generator.dart (right): https://chromiumcodereview.appspot.com/93743006/diff/1/lib/file_generator.dart#newcode135 lib/file_generator.dart:135: out.println("import '${_generatedFilePath(relativeProtoPath)}'"); remove new line ...
7 years ago (2013-12-20 18:27:36 UTC) #2
Søren Gjesse
https://codereview.chromium.org/93743006/diff/1/lib/file_generator.dart File lib/file_generator.dart (right): https://codereview.chromium.org/93743006/diff/1/lib/file_generator.dart#newcode135 lib/file_generator.dart:135: out.println("import '${_generatedFilePath(relativeProtoPath)}'"); On 2013/12/20 18:27:36, Siggi Cherem (dart-lang) wrote: ...
6 years, 11 months ago (2014-01-02 08:52:32 UTC) #3
Søren Gjesse
6 years, 11 months ago (2014-01-02 08:55:10 UTC) #4
Søren Gjesse
Committed patchset #2 manually as r24f06a2 (presubmit successful).
6 years, 11 months ago (2014-01-02 08:56:40 UTC) #5
Siggi Cherem (dart-lang)
6 years, 11 months ago (2014-01-02 18:29:46 UTC) #6
Message was sent while issue was closed.
https://codereview.chromium.org/93743006/diff/1/lib/protobuf_field.dart
File lib/protobuf_field.dart (right):

https://codereview.chromium.org/93743006/diff/1/lib/protobuf_field.dart#newco...
lib/protobuf_field.dart:21: final String fqname;
On 2014/01/02 08:52:33, Søren Gjesse wrote:
> On 2013/12/20 18:27:36, Siggi Cherem (dart-lang) wrote:
> > I know this is unrelated, but what does 'fqname' stand for?
> 
> fqname is the "fully qualified name", that is the field name prefixed by the
> fqname of the class. The fqname of the class is the class name prefoxed by a .
> and the package name (if any).

I see, thanks for the clarification!

I think renaming this to qualifiedName or something similar would totally be
worth it in the future.

Powered by Google App Engine
This is Rietveld 408576698