Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1883093002:
Add support for generating the QtCreator projects from GN. (Closed)
|
Created:
4 years, 8 months ago by Krzysztof Olczyk Modified:
4 years, 7 months ago CC:
chromium-reviews, matthewtff_gmail.com, tfarina Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd support for generating QtCreator projects from GN.
This adds a new command line argument "--ide=" value to "gn gen"
which, when specified, generates a QtCreator project.
QtCreator is a quite powerful general-purpose (despite Qt in the name)
IDE when developing on Linux system with code completion and navigation.
Some interest in it has been demonstrated in the following thread:
https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8
BUG=
Committed: https://crrev.com/af72603e3cd01c17d98c3edf982b064516260bc6
Cr-Commit-Position: refs/heads/master@{#393514}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Rebased on HEAD, fixed issues. #
Total comments: 2
Patch Set 3 : Fixed compilation errors on Windows (paths in Windows :]) #
Total comments: 2
Patch Set 4 : Addressed the final comment #Patch Set 5 : #
Total comments: 2
Messages
Total messages: 33 (13 generated)
kolczyk@opera.com changed reviewers: + brettw@chromium.org, dpranke@chromium.org
Hi, could you please take a look at this CL related to https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 discussion?
Description was changed from ========== Add support for generating the QtCreator projects from GN. This adds a new command line argument's "--ide=" value to "gn gen", which, when specified, generates a QtCreator project. QtCreator is a quite a powerful general-purpose (despite Qt in the name) when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ========== to ========== Add support for generating the QtCreator projects from GN. This adds a new command line argument's "--ide=" value to "gn gen", which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ==========
Description was changed from ========== Add support for generating the QtCreator projects from GN. This adds a new command line argument's "--ide=" value to "gn gen", which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ========== to ========== Add support for generating QtCreator projects from GN. This adds a new command line argument's "--ide=" value to "gn gen", which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ==========
Description was changed from ========== Add support for generating QtCreator projects from GN. This adds a new command line argument's "--ide=" value to "gn gen", which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ========== to ========== Add support for generating QtCreator projects from GN. This adds a new command line argument "--ide=" value to "gn gen" which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ==========
brucedawson@chromium.org changed reviewers: + brucedawson@chromium.org
I made some minor tweaks to the description, change back if you don't like them (extraneous articles mostly). A few comments: https://codereview.chromium.org/1883093002/diff/1/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/1/tools/gn/command_gen.cc#new... tools/gn/command_gen.cc:243: " \"qtcreator\" - QtCreator project directory." This should say "QtCreator project file.s" not "project directory." Also, needs a terminating \n. https://codereview.chromium.org/1883093002/diff/1/tools/gn/qt_creator_writer.cc File tools/gn/qt_creator_writer.cc (right): https://codereview.chromium.org/1883093002/diff/1/tools/gn/qt_creator_writer.... tools/gn/qt_creator_writer.cc:147: std::set<std::string>* items) { Should be const std::set, and perhaps then a reference instead of a pointer?
Thanks Bruce for the review. I have rebased the CL and addressed your comments. https://codereview.chromium.org/1883093002/diff/1/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/1/tools/gn/command_gen.cc#new... tools/gn/command_gen.cc:243: " \"qtcreator\" - QtCreator project directory." On 2016/05/09 at 18:56:49, brucedawson wrote: > This should say "QtCreator project file.s" not "project directory." Also, needs a terminating \n. Done https://codereview.chromium.org/1883093002/diff/1/tools/gn/qt_creator_writer.cc File tools/gn/qt_creator_writer.cc (right): https://codereview.chromium.org/1883093002/diff/1/tools/gn/qt_creator_writer.... tools/gn/qt_creator_writer.cc:147: std::set<std::string>* items) { On 2016/05/09 at 18:56:49, brucedawson wrote: > Should be const std::set, and perhaps then a reference instead of a pointer? Done.
lgtm
The CQ bit was checked by kolczyk@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1883093002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1883093002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
This looks basically okay, but I'd like to run this locally to look at the generated output just to double-check my understanding. However, it looks like this doesn't actually compile (see the try jobs). https://codereview.chromium.org/1883093002/diff/20001/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/20001/tools/gn/command_gen.cc... tools/gn/command_gen.cc:289: " thus build all the targets.\n" I would split this section out to have a separate "QtCreator Flags" section. It's a bit confusing to mention QtCreator in the Xcode section (although it is also a bit silly to have a section for just one flag). Also, I would rewrite this as "If unset, \"All\" invokes ninja without any targets\n" "and builds everything.\n" the grammar is a bit off otherwise.
I have fixed the windows compilation problem and update docs as suggested. https://codereview.chromium.org/1883093002/diff/20001/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/20001/tools/gn/command_gen.cc... tools/gn/command_gen.cc:289: " thus build all the targets.\n" On 2016/05/11 at 15:36:37, Dirk Pranke wrote: > I would split this section out to have a separate "QtCreator Flags" section. It's a bit confusing to mention > QtCreator in the Xcode section (although it is also a bit silly to have a section for just one flag). > > Also, I would rewrite this as > > "If unset, \"All\" invokes ninja without any targets\n" > "and builds everything.\n" > > the grammar is a bit off otherwise. Done.
https://codereview.chromium.org/1883093002/diff/40001/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/40001/tools/gn/command_gen.cc... tools/gn/command_gen.cc:287: " Name of the target corresponding to \"All\" target in Xcode\n" You didn't create a separate "QtCreator Flags" section like I suggested. LGTM if you do that as well (I'm assuming you just forgot), but if you think that's the wrong thing to do, I'd like to know your thoughts.
https://codereview.chromium.org/1883093002/diff/40001/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/40001/tools/gn/command_gen.cc... tools/gn/command_gen.cc:287: " Name of the target corresponding to \"All\" target in Xcode\n" On 2016/05/13 at 01:50:23, Dirk Pranke wrote: > You didn't create a separate "QtCreator Flags" section like I suggested. LGTM if you do that as well (I'm assuming you just forgot), but if you think that's the wrong thing to do, I'd like to know your thoughts. Yes, I totally agree. I apologize - somehow, I haven't read the initial part of your comment. Done now.
The CQ bit was checked by kolczyk@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1883093002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1883093002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by kolczyk@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from brucedawson@chromium.org, dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/1883093002/#ps80001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1883093002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1883093002/80001
Message was sent while issue was closed.
Description was changed from ========== Add support for generating QtCreator projects from GN. This adds a new command line argument "--ide=" value to "gn gen" which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ========== to ========== Add support for generating QtCreator projects from GN. This adds a new command line argument "--ide=" value to "gn gen" which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Add support for generating QtCreator projects from GN. This adds a new command line argument "--ide=" value to "gn gen" which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= ========== to ========== Add support for generating QtCreator projects from GN. This adds a new command line argument "--ide=" value to "gn gen" which, when specified, generates a QtCreator project. QtCreator is a quite powerful general-purpose (despite Qt in the name) IDE when developing on Linux system with code completion and navigation. Some interest in it has been demonstrated in the following thread: https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/9U4_ytjrah8 BUG= Committed: https://crrev.com/af72603e3cd01c17d98c3edf982b064516260bc6 Cr-Commit-Position: refs/heads/master@{#393514} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/af72603e3cd01c17d98c3edf982b064516260bc6 Cr-Commit-Position: refs/heads/master@{#393514}
Message was sent while issue was closed.
https://codereview.chromium.org/1883093002/diff/80001/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/80001/tools/gn/command_gen.cc... tools/gn/command_gen.cc:296: " the whole build graph will be omitted.\n" the whole build graph will be omitted?
Message was sent while issue was closed.
https://codereview.chromium.org/1883093002/diff/80001/tools/gn/command_gen.cc File tools/gn/command_gen.cc (right): https://codereview.chromium.org/1883093002/diff/80001/tools/gn/command_gen.cc... tools/gn/command_gen.cc:296: " the whole build graph will be omitted.\n" On 2016/05/13 15:24:42, Dirk Pranke wrote: > the whole build graph will be omitted? My guess is that "emitted" was the word that was meant here. That's worth fixing. Stupid English language. Whose idea was it to have a pair of words with almost opposite meaning that differ by just one letter? Very poor design.
Message was sent while issue was closed.
On 2016/05/13 16:39:02, brucedawson wrote: > Stupid English language. Whose idea was it to have a pair of words with almost > opposite meaning that differ by just one letter? Very poor design. <dr. nick>inflammable means flammable? what a country!</dr. nick>
Message was sent while issue was closed.
On 2016/05/13 at 17:03:18, dpranke wrote: > On 2016/05/13 16:39:02, brucedawson wrote: > > Stupid English language. Whose idea was it to have a pair of words with almost > > opposite meaning that differ by just one letter? Very poor design. > > <dr. nick>inflammable means flammable? what a country!</dr. nick> Ay. Sorry for the typo, will post a fixup.
Message was sent while issue was closed.
On 2016/05/14 at 07:33:13, Krzysztof Olczyk wrote: > On 2016/05/13 at 17:03:18, dpranke wrote: > > On 2016/05/13 16:39:02, brucedawson wrote: > > > Stupid English language. Whose idea was it to have a pair of words with almost > > > opposite meaning that differ by just one letter? Very poor design. > > > > <dr. nick>inflammable means flammable? what a country!</dr. nick> > > Ay. Sorry for the typo, will post a fixup. I have created https://codereview.chromium.org/1979813002 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
