Community
Participate
Working Groups
See https://wiki.eclipse.org/E4/UI/Smart_Import I think we should start moving it to platform UI.
This was discussed with Dani, and our (JBoss Tools people) plan is to ship it as a separate plugin for JBoss Tools 4.3/JBoss Developer Studio 9.0 release which should happen just after Mars SR1 (so targeting for Mars M2 or M3).
Mickael, any plans to provide a Gerrit review for M3?
Ideally, I'd like someone(s) from Platform/UI to give a try to this feature first on the current site, open bugs, so I can easily fix them in current repo. Then, when we agree it's good enough, we can move it to Platform/UI.
I will not manage to review this in M3, if another committer finds the time, please move back to M3.
I tried https://git.eclipse.org/r/#/c/61233/ with plain SDK for importing Java projects. Here are some comments: - It does not support importing projects from .zip files. - When a zipped project is extracted, it usually has this structure: "P1\P1\...". When the outer directory "P1\" is given as root, it shows 2 projects (P1 and P1\P1) will be imported, but only P1 is imported as a general project and P1\P1 is not imported separately as an Eclipse (Java) project. - After importing, the resulting "Imported Projects" dialog is not shown when "Import raw project (I'll ...)" option is selected on first page. - In cases where the resulting "Imported Projects" dialog is shown on top, you can still focus on the hidden import wizard and make changes in it. As a result, when you focus on the hidden import wizard and press 'Cancel' on it, "Unhandled event loop exception - org.eclipse.swt.SWTException: Widget is disposed" is logged in the error log view. - The size of "Imported Projects" dialog can be reduced. - Title is missing for the import wizard. - Typing invalid root diretory shows the error marker on "Select root directory:" label, which is cut off by the label. Usually, we don't show error marker on label in other import wizards. - It will be good to have a drop down at "Select root directory:" with previously selected directories, similar to other import wizards. - Mnemonics are missing for the new options (labels, Browse botton, radio buttons, link, checkboxes etc.). Some high-level code related comments, which can be updated after functional changes are done: - Unused NLS: EasymportWizardPage_availableDetectors in org.eclipse.ui.internal.wizards.datatransfer.messages - @since tags and Javadoc missing for new API classes RecursiveFileFinder and ProjectConfigurator. - Copyright year can be updated to 2016 in all the files. - Code needs to be cleaned-up. Check all warnings like unused parameters, unnecessary thrown exceptions, unchecked type conversions, missing Javadocs etc. - New messages need to be reviewed, like "... projects(BEWARE: ..." should have space before '(' and the message need not be enclosed in '*'.
Thanks Noopur! Before we get into details, what's the general feeling on this feature? Do you find it useful and easy to understand? Would you rather use this to import one or more projects (not necessarily Eclipse ones) than the current Import wizard? (In reply to Noopur Gupta from comment #5) > - It does not support importing projects from .zip files. I'm always surprised when I'm told that the IDE should also support import of zip files whereas extracting a zip is a trivial task on all OSs... For years I've used Eclipse, I don't recall I imported a zip once. Anyway, I'll try to provide that in a next patch set. > - When a zipped project is extracted, it usually has this structure: > "P1\P1\...". When the outer directory "P1\" is given as root, it shows 2 > projects (P1 and P1\P1) will be imported, but only P1 is imported as a > general project and P1\P1 is not imported separately as an Eclipse (Java) > project. I'm not sure I understand clearly, can you please attach the zip so I can investigate that. > - After importing, the resulting "Imported Projects" dialog is not shown > when "Import raw project (I'll ...)" option is selected on first page. > - In cases where the resulting "Imported Projects" dialog is shown on top, > you can still focus on the hidden import wizard and make changes in it. As a > result, when you focus on the hidden import wizard and press 'Cancel' on it, > "Unhandled event loop exception - org.eclipse.swt.SWTException: Widget is > disposed" is logged in the error log view. > - The size of "Imported Projects" dialog can be reduced. > - Title is missing for the import wizard. I'll take care of them. > - Typing invalid root diretory shows the error marker on "Select root > directory:" label, which is cut off by the label. Usually, we don't show > error marker on label in other import wizards. I like a lot error markers, they are very efficient to immediately identify an error, more than other error mechanism. So I'd like to keep it. However, if you think it's not correctly placed. I'd be glad to fix that. Isn't this marker actually on the Text Field? > - It will be good to have a drop down at "Select root directory:" with > previously selected directories, similar to other import wizards. I'm wondering what would make the user have to import the same thing multiple times (almost) in a raw to take advantage of this widget. But I'll put a Combo in a next patch set anyway, for the sake of consistency. > - Mnemonics are missing for the new options (labels, Browse botton, radio > buttons, link, checkboxes etc.). > Some high-level code related comments, which can be updated after functional > changes are done: > - Unused NLS: EasymportWizardPage_availableDetectors in > org.eclipse.ui.internal.wizards.datatransfer.messages > - @since tags and Javadoc missing for new API classes RecursiveFileFinder > and ProjectConfigurator. > - Copyright year can be updated to 2016 in all the files. > - Code needs to be cleaned-up. Check all warnings like unused parameters, > unnecessary thrown exceptions, unchecked type conversions, missing Javadocs > etc. > - New messages need to be reviewed, like "... projects(BEWARE: ..." should > have space before '(' and the message need not be enclosed in '*'. I'll take care of this.
> - Mnemonics are missing for the new options (labels, Browse botton, radio > buttons, link, checkboxes etc.). Are there some recommendation/guidelines to choose mnemonics?
I also tested it. The options on the first import page were confusing for me. Currently you have: Import raw project (I'll configure it later) Import and configure root project only Detect and configure nested projects under the given location As toogles. I suggest to change this to checkboxes: Search for nested project Avoid configuring project natures This gives you all three options but are simpler to read. I think we should also add the existing options from the "old" import wizard. Copy projects into workspace Hide projects that already exists in the workspace Especially the "Hide" option is extremely useful to me. As you now also supporting the import from Archive files I suggest to adjust the description of the dialog. --------- This operation will analysis the content of your folder-> This wizard analysis the content of your folder or archive file Import projects from folder -> Import projects ----------
(In reply to Lars Vogel from comment #8) > I suggest to change this to checkboxes: > > Search for nested project > Avoid configuring project natures Ok. I'll try that. Just I'd rather have a "positive" assertion in checkboxes, so I'll try "Configure project natures" instead. > I think we should also add the existing options from the "old" import wizard. > Copy projects into workspace > Hide projects that already exists in the workspace Do you think those can be part of a next iteration, or are they blocker for the review. Those options would be one the "proposals" page, right? > As you now also supporting the import from Archive files I suggest to adjust > the description of the dialog. > --------- > This operation will analysis the content of your folder-> > This wizard analysis the content of your folder or archive file > > Import projects from folder -> Import projects > ---------- Ok.
(In reply to Mickael Istria from comment #9) > Do you think those can be part of a next iteration, or are they blocker for > the review. They are really, really useful. I would say they are not blocking but you should get them done in the next iteration. > Those options would be one the "proposals" page, right? +1
(In reply to Lars Vogel from comment #10) > (In reply to Mickael Istria from comment #9) > > Do you think those can be part of a next iteration, or are they blocker for > > the review. > > They are really, really useful. I would say they are not blocking but you > should get them done in the next iteration. Ok. I will consider them immediately after the current patch is merged. Could you open a separate report for them. By the way, I've been using this wizard for about a year now, for various kinds of projects, and I never felt the need for those options. So if you can put a few user stories in the bug, that may help me to understand what's the issue and to figure out a good presentation for the solution.
(In reply to Mickael Istria from comment #11) > Ok. I will consider them immediately after the current patch is merged. > Could you open a separate report for them. Bug 487298 > By the way, I've been using this wizard for about a year now, for various > kinds of projects, and I never felt the need for those options. Every user workflow is different.
What are the plans for the existing import wizards, like General -> Archive File, General -> Existing Projects into Workspace and General -> File System? I think we should ensure the new wizard cover their use cases and afterwards retire them. Mickael, is that in sync with your plans?
(In reply to Lars Vogel from comment #13) > What are the plans for the existing import wizards, like General -> Archive > File, General -> Existing Projects into Workspace and General -> File System? > I think we should ensure the new wizard cover their use cases and afterwards > retire them. Mickael, is that in sync with your plans? That would be the ideal, but I am not convinced this will ever be possible because of the general backward-compatibility constraint of Platform. Moreover, as you mentioned in another bug, every user have their workflow; and despite we try, I'm pretty sure that some will prefer the "legacy" import workflow to the new one. The most probable scenario is that user start using in majority the new import wizard, and that it will become the main entry-point for import; but enough would still use the older ones to make it worth keeping them (especially since they don't cost most in the UI, hidden under the "Import" menu).
More feedback: 1.) Second wizard page (Import project) needs a filter box. 2.) As a user I don't understand the "Use additional analysis..." box. Is that not covered by the "Search for nestled projects" box on the first page? 3.) I think the "Working Sets" box on the first page looks a bit squeezed. Can you add an empty line after the last checkbox? 4.) Should the advanced user have the option to disable a detector? For example, maybe he does not want to use the Maven detector even though the tooling is installed? 5.) Can we use the number of detected projects as status reporting number for the SmartImportJobReportDialog? Currently it uses an infinite job reporting, but as a user I prefer to see the progress.
(In reply to Mickael Istria from comment #14) > That would be the ideal, but I am not convinced this will ever be possible > because of the general backward-compatibility constraint of Platform. We can and should remove inferior options from the UI, if we have better options. For example, once Bug 487298 is implemented we should remove the "Existing Projects into Workspace" option. Maybe not for Neon but definitely for the Oxygen release. On a related note, I suggest to rename your UI action to "Existing Projects from File System".
(In reply to Lars Vogel from comment #15) > 1.) Second wizard page (Import project) needs a filter box. Ok. > 2.) As a user I don't understand the "Use additional analysis..." box. Is > that not covered by the "Search for nestled projects" box on the first page? No, this part is not covered by the detection. I didn't manage to find a good way to explain it yet, but I'll try again, and maybe we can come up with a better label: I imagined a few detectors cannot (yet?) process on the "first-round" analysis on file-system, but can infer additional structure once projects are imported into workspace. It can be the case for some JavaEE apps which aren't Maven-based for example. So in those cases, the checkbox proposes to also run those detectors after import. > 3.) I think the "Working Sets" box on the first page looks a bit squeezed. > Can you add an empty line after the last checkbox? Yep. > 4.) Should the advanced user have the option to disable a detector? For > example, maybe he does not want to use the Maven detector even though the > tooling is installed? I thought about it but at the moment, it doesn't seem to be something useful. I believe it's more something to keep in mind and more usage report will drive us to the right conclusion. Ideally, users will never want to do that. Everything is just imported as best, and users like what's best. > 5.) Can we use the number of detected projects as status reporting number > for the > SmartImportJobReportDialog? Currently it uses an infinite job reporting, but > as a user I prefer to see the progress. Good idea. I'll do that.
(In reply to Mickael Istria from comment #17) > I thought about it but at the moment, it doesn't seem to be something > useful. I think this would also be a user protecting for badly implemented importers or conflicting or undesired ones. For example, I have the Maven tooling installed but for eclipse.platform.ui projects I see no advantage in configuring the projects via Maven so I want to skip that. Or a configurer takes very, very long (Maven comes to mind again) and I want to have fast look at the project. Instead of "Show available detectors" you can make it "Configure available detectors" and make them selectable. A detectors should also have a small description which should be visible somewhere to the user. For example, as a user I want to know what the workspace detector does.
(In reply to Lars Vogel from comment #18) > I think this would also be a user protecting for badly implemented importers > or conflicting or undesired ones. By the way, users will see that a detector is badly implemented once the damage is done. IMO, we need to reconsider ability to set project natures (bug 102527 for which I have a proposal). > For example, I have the Maven tooling > installed but for eclipse.platform.ui projects I see no advantage in > configuring the projects via Maven so I want to skip that. Or a configurer > takes very, very long (Maven comes to mind again) and I want to have fast > look at the project. If you import project from eclipse.platform.ui, then projects are detected as eclipse projects, because they have a .project; and all other kind of detection is skipped. > Instead of "Show available detectors" you can make it "Configure available > detectors" and make them selectable. > A detectors should also have a small description which should be visible > somewhere to the user. For example, as a user I want to know what the > workspace detector does. Ok. I'll try to fix it. However, I don't think it is a blocking one nor a very big differentiator for user experience right now; so it may become part of a 2nd round.
(In reply to Mickael Istria from comment #19) > Ok. I'll try to fix it. However, I don't think it is a blocking one nor a > very big differentiator for user experience right now; so it may become part > of a 2nd round. Sure, feel free to move that to a new bug report.
Mickael, please update this bug report once you have implemented the suggestions or opened new bug reports for them so that I can review the changes. I would like to see this new import wizard in for early M6 if possible.
(In reply to Mickael Istria from comment #19) > If you import project from eclipse.platform.ui, then projects are detected > as eclipse projects, because they have a .project; and all other kind of > detection is skipped. Is that the desired outcome? For example, I could image that EGit wants to add the Git repository into the Git repository view, even if the .project file exists in the imported project.
(In reply to Lars Vogel from comment #22) > Is that the desired outcome? For example, I could image that EGit wants to > add the Git repository into the Git repository view, even if the .project > file exists in the imported project. Yes, I'll do that. Unfortunately, it's going to be tricky: I'm busy all week with a team meeting and I'll take a week of holidays after that. I won't fully available for that from Feb 22nd.
I've updated the Gerrit patch set to introduce a better progress report. There are still some interesting items in all your feedback that I didn't have the opportunity to handle. I won't be available on the next week. So if you feel it's not too far from being merged and can fix some small things that are necessary for this 1st step, feel free to change the code and/or append commits; don't wait for me to come back.
Created attachment 259906 [details] Screenshot - error marker I tried patch set 10 and observed the following: - When selecting "Import Projects..." option in File menu, we first get the dialog to select a folder. This step looks unnecessary. The dialog can also be invoked by selecting "Directory..." in the new wizard. The behavior should be same when selecting the menu entry or the new option in existing Import wizard. - Got this exception while going to next page in the wizard. First I got multiple dialogs showing the same error about resource being used by another process and then this NPE: java.lang.NullPointerException at org.eclipse.ui.internal.wizards.datatransfer.SmartImportProposalsWizardPage.pageChanged(SmartImportProposalsWizardPage.java:353) at org.eclipse.jface.wizard.WizardDialog$6.run(WizardDialog.java:1491) - The final "Imported Projects" dialog always has the red "Abort" button enabled even after completion. - I imported a zipped project (from bug 484686) and got two corresponding projects in the workspace. I selected both, pressed Delete and checked "Delete project contents on disk". Pressing OK shows the warning that projects contain resources that are not in sync with the projects. Continue and delete the projects. Now, again import the same and try to delete with above steps. We get exceptions like: Problems encountered while deleting files. Could not delete: C:\ProjectWork\runtime-SmartImporter\jdt484686.zip_expanded\jdt484686\pom.xml. The process cannot access the file because it is being used by another process. - Mnemonics are missing for: * "Import Projects..." menu entry in File menu. * "Use additional analysis..." checkbox on "Import proposal" wizard page. - "Analysis" => should be "Analyses" in: * Description of the new option in existing Import wizard. * Description on the first wizard page. - The new option in existing Import wizard should have capital "A" in "archive". (In reply to Lars Vogel from comment #15) > 2.) As a user I don't understand the "Use additional analysis..." box. Same here. (In reply to Noopur Gupta from comment #5) > - Typing invalid root directory shows the error marker... > which is cut off by the label. Attaching the screenshot.
(In reply to Noopur Gupta from comment #25) > - When selecting "Import Projects..." option in File menu, we first get the > dialog to select a folder. This step looks unnecessary. The dialog can also > be invoked by selecting "Directory..." in the new wizard. The behavior > should be same when selecting the menu entry or the new option in existing > Import wizard. If you consider that the number of clicks is a usability/accessibility metrics, than it makes more sense to first ask for the director before showing the wizard. FYI, this is inspired by NetBeans "open" action. > - Got this exception while going to next page in the wizard. First I got > multiple dialogs showing the same error about resource being used by another > process and then this NPE: > java.lang.NullPointerException > at > org.eclipse.ui.internal.wizards.datatransfer.SmartImportProposalsWizardPage. > pageChanged(SmartImportProposalsWizardPage.java:353) > at org.eclipse.jface.wizard.WizardDialog$6.run(WizardDialog.java:1491) Ok, I didn't see that. I'll fix it. > - The final "Imported Projects" dialog always has the red "Abort" button > enabled even after completion. OK. > - I imported a zipped project (from bug 484686) and got two corresponding > projects in the workspace. I selected both, pressed Delete and checked > "Delete project contents on disk". Pressing OK shows the warning that > projects contain resources that are not in sync with the projects. Continue > and delete the projects. Now, again import the same and try to delete with > above steps. We get exceptions like: > Problems encountered while deleting files. > Could not delete: > C:\ProjectWork\runtime-SmartImporter\jdt484686.zip_expanded\jdt484686\pom. > xml. > The process cannot access the file because it is being used by another > process. Ok, strange. I guess it's a stream that's not well closed. If you have any idea of another possible reason, I'm all ears. > - Mnemonics are missing for: > * "Import Projects..." menu entry in File menu. > * "Use additional analysis..." checkbox on "Import proposal" wizard page. Ok. > - "Analysis" => should be "Analyses" in: > * Description of the new option in existing Import wizard. > * Description on the first wizard page. Ok. > - The new option in existing Import wizard should have capital "A" in > "archive". Ok. > (In reply to Lars Vogel from comment #15) > > 2.) As a user I don't understand the "Use additional analysis..." box. > Same here. I'm still open to better suggestions ;). > > (In reply to Noopur Gupta from comment #5) > > - Typing invalid root directory shows the error marker... > > which is cut off by the label. > Attaching the screenshot. Thanks. PS: In general, I find it easier to discuss about code on Gerrit. If it's fine with you, we could use Gerrit for discussion specific to suggested patch set.
First, I want to say thank you Mickael for tackling this problem. It's easy to forget how foreign using "Import" can be to users. I'll say up front that my hopes for this effort should render the Import Wizard unnecessary for most of our user base. I've witnessed developer confusion and incredulity with importing an existing Tycho-based project as a Maven project when they should have done so as an Existing project. Sorting through the different import wizards can be overwhelming for some. I'm also a heavy user of the Import Existing Projects wizard. So much so that I bind it to a key. I concentrated on the flows using three usecases: 1. a committer seeking to import projects, such as example projects attached to reported bugs 2. developer teams seeking to import checkouts that are already on disk 3. a newcomer to Eclipse who has some existing project that they're trying to bring in Here's a summary of what I saw that was a bit jarring: A. 'Import Projects…' is out of position on the File menu B. The immediate directory dialog feels strange C. The import went too deep on my Maven project, pulling out generated projects found under target/ directories D. the inability to provide any control over the detectors or to be able to predict the outcome, especially the 'additional analysis' I liked that it didn't traverse .git/objects directory like the Import Existing Project wizard! But it did traverse into target directories of my Maven/Tycho projects and identify some of those as projects. A. 'Import Projects…' is out of position on the File menu I actually think 'Import Projects…' is misnamed: it should be 'Open Projects…'. With that change, then your menu location is right. I think that matches most users' intentions and their knowledge from other applications (and you mentioned NetBeans uses that terminology too). That also nicely differentiates your effort from the existing Import wizards, which know how to bring in other non-Eclipse artefacts like EARs, XML Catalogs, etc. B. The immediate directory dialog feels strange I agree with Noopur's point that the directory dialog is a bit jarring for the committer-import usecase when you're trying to import a zip, especially since hitting Cancel cancels the import. Thinking about it, I think the logical solution is to have 'Open File…' call into this Smart Import/Smart Open too. There's no reason that 'Open File…' has to open the file in an editor if we can figure out something better. It's unfortunate that SWT doesn't have a FileOrDirectoryDialog, as some platforms do support that (OS X). C, D. The inability to control the experience I found the first page of the wizard felt unnecessary: I'd provided a directory already, why didn't the importer do a trial import and present me with what it found? I like that about the Import Existing Projects wizard. I didn't understand the 'Configure project natures'. If I had existing Eclipse projects, they had natures. If they weren't existing projects, then importing them would require adding natures (no?). I was leery of the additional analyzers, especially given the language described. It goes to what Lars described above in comment 18: if it's already an Eclipse project, just bring it in and don't do anything further without my consent. I think this can be best described as adhering to the principle of least surprise. Allow me to blue-sky a bit, but what I'd really want of a Smart Importer/Opener is a wizard that provided a first-page 'Finish' experience, like the Import Existing Projects. So the wizard would be trying to answer "What is this selected item and what is the easiest action to bring whatever it is into the workspace?" If I provided a directory containing existing Eclipse projects, hitting Finish would detect and suggest import them as Eclipse projects. If they were pure Maven projects (i.e., never previously imported to Eclipse), hitting Finish would import them using m2e or some other Maven importer. And I should be able to switch things out — specify that this Eclipse-bearing Maven project *should* be imported using m2e rather than as a native Eclipse project. So perhaps the first page has the list of detectors that matched, so I could unselect the ones I'm not interested in (e.g., the native Eclipse project detector). The additional natures / extra analyzers could be done as a second page in the wizard. I'd also like the importer to have a small catalog of project types. So I could point to a pure Maven project, but without m2eclipse installed, and have the importer prompt me to install m2e. So to summarize: 1. I think this should be thought of as the Smart Opener 2. I think we should replace the Open File with this too 3. The wizard should aim to provide a first-page-with-Finish
(In reply to Brian de Alwis from comment #27) > I'll say up front that my hopes for this effort should render the Import > Wizard unnecessary for most of our user base. That's actually my/JBoss Tools crew goal too. > I've witnessed developer > confusion and incredulity with importing an existing Tycho-based project as > a Maven project when they should have done so as an Existing project. > Sorting through the different import wizards can be overwhelming for some. Same with some other kind of projects. Importing a project in Eclipse is way too confusing and difficult for a very early usage. > 1. a committer seeking to import projects, such as example projects > attached to reported bugs > 2. developer teams seeking to import checkouts that are already on disk > 3. a newcomer to Eclipse who has some existing project that they're trying > to bring in FYI, I concentrate more on 2 and 3, since I consider that users in category 1 are advanced users and that they understand and know the IDE better to not require that much assistance. 2 and 3 are the priority, but of course the workflow should be also useful to category 1; just it shouldn't be more complex to newcomers and regular IDE users "just" for Eclipse committers. > it should be 'Open Projects…'. With that change, then your menu location is right. I'd be very glad to have the menu called "Open Projects". > B. The immediate directory dialog feels strange > > I agree with Noopur's point that the directory dialog is a bit jarring for > the committer-import usecase when you're trying to import a zip, especially > since hitting Cancel cancels the import. As you said, it's jarring for the committer-import of a zip; but I don't think it's the user-story that we should base the design on (as I explained above). > Thinking about it, I think the logical solution is to have 'Open File…' call > into this Smart Import/Smart Open too. There's no reason that 'Open File…' > has to open the file in an editor if we can figure out something better. At the moment, opening a file is not part of the user story. I don't find a strong relationship between opening a file and opening a project. We could imagine a composite menu "Open >" with 2 sub-menus File and Projects. But I'm not sure I actually like it better than a separate menu under File. > C. The import went too deep on my Maven project, pulling out generated projects found under target/ directories This is actually a bug in the Maven strategy. I can fix it right away in m2e. Don't consider it as a bug in the Smart Import/Opener. > D. The inability to control the experience > I found the first page of the wizard felt unnecessary: I'd provided a > directory already, why didn't the importer do a trial import and present me > with what it found? Ok, that makes sense to run the preliminary analysis early and to show proposal right away. > I didn't understand the 'Configure project natures'. If I had existing > Eclipse projects, they had natures. If they weren't existing projects, then > importing them would require adding natures (no?). Ok, that's misleading for existing Eclipse projects. What about "Configure project natures for non-Eclipse projects" or maybe "Detect project type" which is more abstract? What I liked with using the word nature is that we softly introduce that Eclipse specific critical word to user when it makes sense, so they learn the word right away. > I was leery of the additional analyzers, especially given the language > described. It goes to what Lars described above in comment 18: if it's > already an Eclipse project, just bring it in and don't do anything further > without my consent. Ok, we definitely need to find a better label. Unless I missed something, Eclipse projects are imported "as it" without a change. At least, that's what it should do, maybe you found a bug. If so, can you please share steps to reproduce? > I think this can be best described as adhering to the principle of least surprise. I didn't know that principle by like it a lot ;) > And I should be able to switch things out — specify > that this Eclipse-bearing Maven project *should* be imported using m2e > rather than as a native Eclipse project. If this is a real use-case, then I don't think it's important enough to be considered right away. Users just want something that works without tweaking and this import mechanism must provide that without requiring users to tweak stuff. IMO, it's simply not worth it. > I'd also like the importer to have a small catalog of project types. So I > could point to a pure Maven project, but without m2eclipse installed, and > have the importer prompt me to install m2e. Yes, I thought about it. But it's not something that I believe we can implement so close to the API Freeze. > 1. I think this should be thought of as the Smart Opener It's actually how I see it, I just adopted the "Import" word to use the "principle of least surprise" to other contributors. > 2. I think we should replace the Open File with this too I didn't manage to understand the benefit for end-users and how would such a generic "open" would properly handle both File and Folders. > 3. The wizard should aim to provide a first-page-with-Finish OK.
Created attachment 260205 [details] import never ending I gave more feedback on the Gerrit. I really wanted to have this in for M6, but the import from eclipse.platform.ui seems to break the import wizard and I think this needs to work before merging the wizard. Also the Cancel button does not seem to work. See screenshot.
(In reply to Lars Vogel from comment #29) > but the import from eclipse.platform.ui seems to break the import wizard Can you elaborate on what actually breaks?
Mickael, once eclipse.platform.ui import works (it does not yet) I'm +2 for merging this for M6 and have it visible in the ui to get more user testing, if you can confirm that you will be available to fix issued discovered after the merge.
(In reply to Mickael Istria from comment #30) > (In reply to Lars Vogel from comment #29) > > but the import from eclipse.platform.ui seems to break the import wizard > > Can you elaborate on what actually breaks? Wizard never finishes, see screenshot. Did you try importing eclipse.platform.ui?
@Mickael, I will be offline soon for a bit more than a week, so if you manage to fix the wizard so that it can import eclipse.platform.ui and (to have other examples eclipse.jdt.ui and eclipse.pde.ui) via the wizard without any issues, I'm OK with the integration of the new wizard for M6. This assumes that you still will to be available to fix issued discovered after the merge.
(In reply to Lars Vogel Unavailable until 21.03.2016 from comment #33) > @Mickael, I will be offline soon for a bit more than a week, so if you > manage to fix the wizard so that it can import eclipse.platform.ui and (to > have other examples eclipse.jdt.ui and eclipse.pde.ui) via the wizard > without any issues, I'm OK with the integration of the new wizard for M6. Ok, I fixed the issue with the persistent report dialog, and I'm about to merge it. > This assumes that you still will to be available to fix issued discovered > after the merge. I am still available to fix that. It has been my top priority task for the last month and will remain until there is nothing to do on it. I really want it to reach a very high quality for Neon release. So future feedback will always be welcome. For further work, once this is merged in M6, I would prefer that we open individual tickets for reports and suggestions. It will make tracking and discussion much easier. I will work on other projects (EGit, m2e, JDT, JSDT, PDE...) to integrate their specific project detectors and configurators strategy to this wizard.
Gerrit change https://git.eclipse.org/r/61233 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5cf373f08d312b875f7671c758f3eb3c6d46a143
Lars approved the change and it was merged. There is still room for improvement for this feature, but let's close this ticket and open separate ones for next changes.
Javadoc problems in http://download.eclipse.org/eclipse/downloads/drops4/I20160314-2000/compilelogs/platform.doc.isv.javadoc.txt would already have been visible in the IDE. Don't commit new code with problems.
New Gerrit change created: https://git.eclipse.org/r/68431
At least ProjectConfigurator is not an Eclipse Quality API. If you want to keep your committer status, this must improve! /** * This method MUST BE stateless (ideally static) * @param folder * @param monitor * @return true if the given folder is for sure to be considered as a project */ public boolean shouldBeAnEclipseProject(IContainer container, IProgressMonitor monitor);
(In reply to Markus Keller from comment #39) > At least ProjectConfigurator is not an Eclipse Quality API. If you want to > keep your committer status, this must improve! I've been asking for a review multiple times, for several months. I would have loved to hear such feedback before the API Freeze...
(In reply to Mickael Istria from comment #40) > I've been asking for a review multiple times, for several months. I would > have loved to hear such feedback before the API Freeze... Mickael, some of these were already mentioned in comment #5. (In reply to Noopur Gupta from comment #5) > > Some high-level code related comments, which can be updated after > > functional changes are done:
(In reply to Noopur Gupta from comment #41) > (In reply to Mickael Istria from comment #40) > > I've been asking for a review multiple times, for several months. I would > > have loved to hear such feedback before the API Freeze... > > Mickael, some of these were already mentioned in comment #5. Indeed, I took them into account but I admit I missed a few of them. However, I'm not sure those are the ones Markus refers to in his last comment.
Gerrit change https://git.eclipse.org/r/68431 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9f6e138b72eadf6b34168a24ffd35b7714eaa11c
"This method MUST BE be stateless (ideally static)" is not an API. There's abundant literature on how to write Javadoc comments. E.g. that the first sentence needs to describe the essence of the API. I'm afraid you won't find people here that have the resources to give you a free course on software engineering. Reviews are meant to find non-obvious problems *after* you've already done a thorough self-review. They're not a device to get others to do your job. ProjectConfigurator#getConfigurationWizard() is a deprecated method with an outdated comment. How can you release something like this as an API? It's a shame that you even need to bother the PMC now to request removal of that method in M7.
(In reply to Markus Keller from comment #44) I'm used to developing software in more permissive contexts, focusing on the value delivered to users over code constraints. So I admit the code I write can look unclean to you and others, and to conform to Platform UI codestyle. I apologize about that. However, I'd be glad to improve myself on this topic; and I wished the code review could be used for this. Mind you that many people, teams and organizations consider code-review as a good way of learning, teaching and collaborating as well than a tool to focus on details. I adhere to this vision and imagined it would be the same for Platform UI in general. However, your comment tells me I had hoped to much. My bad. I'll do what's necessary to make this code good enough for Platform UI before Neon release. However, I concretely do not know exactly what that means and what's to do. Any hint and recommendation is welcome.
(In reply to Mickael Istria from comment #45) > (In reply to Markus Keller from comment #44) > > Mind you that many people, teams and > organizations consider code-review as a good way of learning, teaching and > collaborating as well than a tool to focus on details. I adhere to this > vision and imagined it would be the same for Platform UI in general. > However, your comment tells me I had hoped to much. My bad. Markus comments do not express an official policy of Platform UI. I agree with your point of view and I think the majority of other Platform UI committers do also. IMHO any significant enhancement will need some polish, this also happens in other projects like JDT.
(In reply to Markus Keller from comment #44) > "This method MUST BE be stateless (ideally static)" is not an API. There's > abundant literature on how to write Javadoc comments. E.g. that the first > sentence needs to describe the essence of the API. I'm afraid you won't find > people here that have the resources to give you a free course on software > engineering. Reviews are meant to find non-obvious problems *after* you've > already done a thorough self-review. They're not a device to get others to > do your job. > > ProjectConfigurator#getConfigurationWizard() is a deprecated method with an > outdated comment. How can you release something like this as an API? It's a > shame that you even need to bother the PMC now to request removal of that > method in M7. I opened Bug 472614 for the polish of the new project importer. I suggest to update this 472614 or open new bugs for new issues.
Following Lars suggestion, let's close this one and keep it in the M6 bucket and track improvements improvements in bug 490361 in "dependent" ones.
