Migrate project to use IntelliJ instead of Eclipse #83 #84

yamgent · 2017-08-16T01:27:38Z

Fixes #83.

As we are moving to IntelliJ, the Eclipse project files are no longer needed. Let's remove them.

mention-bot · 2017-08-16T01:27:39Z

@yamgent, thanks for your PR! By analyzing the history of the files in this pull request, we identified @damithc, @louietyj and @weikangchia to be potential reviewers.

CanIHasReview-bot · 2017-08-16T01:29:47Z

v1

@yamgent submitted v1 for review.

[v1 1/2] Remove Eclipse project files
[v1 2/2] DeveloperGuide: Migrate setup instructions to IntelliJ

(📚 Archive)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/84/1/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

CanIHasReview-bot · 2017-08-16T01:33:46Z

v2

@yamgent submitted v2 for review.

[v2 1/2] Remove Eclipse project files
[v2 2/2] DeveloperGuide: Migrate setup instructions to IntelliJ

(📚 Archive) (📈 Interdiff between v1 and v2)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/84/2/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

yamgent · 2017-08-16T01:34:26Z

doc/DeveloperGuide.md

+   2. Press <kbd>ALT</kbd>+<kbd>ENTER</kbd> and select `Add 'JUnit4' to classpath`
+   3. Select `Use 'JUnit4' from IntelliJ IDEA distribution` and click `OK`
+10. Set the working directory of tests to the project directory
+    1. Ensure that you have run all the tests cases (if not, right-click the `test` folder, and click `Run 'All Tests'`)


Had to indent one more space, because GitHub doesn't render this inner list properly otherwise.

There is a separate section for testing later in this doc, which is still written for Eclipse BTW. Do a project-wide search for Eclipse to be safe.

damithc · 2017-08-16T02:24:51Z

doc/DeveloperGuide.md

+   2. Press <kbd>ALT</kbd>+<kbd>ENTER</kbd> and select `Add 'JUnit4' to classpath`
+   3. Select `Use 'JUnit4' from IntelliJ IDEA distribution` and click `OK`
+10. Set the working directory of tests to the project directory
+    1. Ensure that you have run all the tests cases (if not, right-click the `test` folder, and click `Run 'All Tests'`)


There is a separate section for testing later in this doc, which is still written for Eclipse BTW. Do a project-wide search for Eclipse to be safe.

CanIHasReview-bot · 2017-08-16T14:18:25Z

v3

@yamgent submitted v3 for review.

[v3 1/3] Remove Eclipse project files
[v3 2/3] DeveloperGuide: Migrate setup instructions to IntelliJ
[v3 3/3] UserGuide: Migrate instructions to IntelliJ

(📚 Archive) (📈 Interdiff between v2 and v3)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/84/3/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

yamgent · 2017-08-16T14:20:39Z

doc/UserGuide.md

-> When running the program inside Eclipse, you can 
-  [set command line parameters before running the program](http://stackoverflow.com/questions/7574543/how-to-pass-console-arguments-to-application-in-eclipse).
+> When running the program inside IntelliJ, you can
+  [set command line parameters before running the program](https://stackoverflow.com/a/2066465).


On a side note, I don't think we have this feature in addressbook-level3. I looked around the code and there is nowhere that reads in the command line parameter and pass that into StorageFile(String filePath).

Can remove in that case.

Ok, I also created issue #85 about this.

yamgent · 2017-08-16T14:22:07Z

Also should we update LearningOutcomes.md to point to the tutorial's intelliJ version?

damithc · 2017-08-16T14:56:45Z

Also should we update LearningOutcomes.md to point to the tutorial's intelliJ version?

Yup.

CanIHasReview-bot · 2017-08-17T01:48:25Z

v4

@yamgent submitted v4 for review.

[v4 1/5] Remove Eclipse project files
[v4 2/5] DeveloperGuide: Migrate setup instructions to IntelliJ
[v4 3/5] UserGuide: Migrate instructions to IntelliJ
[v4 4/5] UserGuide: Remove section "Changing Save Location"
[v4 5/5] LearningOutcomes: Update JavaFx tutorial to IntelliJ version

(📚 Archive) (📈 Interdiff between v3 and v4)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/84/4/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

damithc

Looks good. Just a couple of cosmetic suggestions.

damithc · 2017-08-17T14:49:17Z

doc/DeveloperGuide.md

+   2. Press <kbd>ALT</kbd>+<kbd>ENTER</kbd> and select `Add 'JUnit4' to classpath`
+   3. Select `Use 'JUnit4' from IntelliJ IDEA distribution` and click `OK`
+10. Set the working directory of tests to the project directory
+    1. Ensure that you have run all the tests cases (if not, right-click the `test` folder, and click `Run 'All Tests'`)


This sentence comes as a surprise. Is running test cases part of the set up process. I don't see any instructions to that effect prior to this sentence.

damithc · 2017-08-17T14:50:05Z

doc/LearningOutcomes.md

@@ -84,11 +84,11 @@ Covered by `[LO-Polymorphism]`

 **Resources**

-* [JavaFX 8 Tutorial](http://code.makery.ch/library/javafx-8-tutorial/) by Marco Jakob
+* [JavaFX 8 Tutorial](https://se-edu.github.io/se-book/javaTools/javaFXBasic/) adapted with permission from Marco Jakob


No need to mention adapted with permission from Marco Jakob here as it is already mentioned in the tutorial itself.

CanIHasReview-bot · 2017-08-19T03:45:59Z

v5

@yamgent submitted v5 for review.

[v5 1/5] Remove Eclipse project files
[v5 2/5] DeveloperGuide: Migrate setup instructions to IntelliJ
[v5 3/5] UserGuide: Migrate instructions to IntelliJ
[v5 4/5] UserGuide: Remove section "Changing Save Location"
[v5 5/5] LearningOutcomes: Update JavaFx tutorial to IntelliJ version

(📚 Archive) (📈 Interdiff between v4 and v5)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/84/5/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

damithc · 2017-08-19T03:53:09Z

doc/DeveloperGuide.md

+2. Go to `Run` -> `Edit Configurations...`
+3. On the list at the left, ensure that `All in test` is selected
+4. Under `Configuration`, change the `Working directory` to the `addressbook-level3` folder
+5. Click `OK`


How about this?

10. Run all the tests (right-click the `test` folder, and click `Run 'All Tests'`) 11. Observe how some tests fail. That is because they try to access the test data from the wrong directory (the working directory is expected to be the root directory, but IntelliJ runs the test with `test\` as the working directory by default). To fix this issue: 1. Go to `Run` -> `Edit Configurations...` 2. On the list at the left, ensure that `All in test` is selected 3. Under `Configuration`, change the `Working directory` to the `addressbook-level3` folder 4. Click `OK` 12. Run the tests again to ensure they all pass now.

We are no longer using Eclipse as our development environment. The instructions in the developer guide are outdated. Let's migrate the setup instructions to IntelliJ.

We are no longer using Eclipse as our development environment. The instructions in the user guide are outdated. Let's migrate the instructions to IntelliJ.

The UserGuide claims that the save location of the addressbook data can be changed via command line parameters. That feature is not fully implemented. As such, addressbook data is always saved in addressbook.txt and cannot be changed. Let's remove the section "Changing Save Location". We can restore this section after the feature is fully implemented.

CanIHasReview-bot · 2017-08-19T05:17:06Z

v6

@yamgent submitted v6 for review.

[v6 1/5] Remove Eclipse project files
[v6 2/5] DeveloperGuide: Migrate setup instructions to IntelliJ
[v6 3/5] UserGuide: Migrate instructions to IntelliJ
[v6 4/5] UserGuide: Remove section "Changing Save Location"
[v6 5/5] LearningOutcomes: Update JavaFx tutorial to IntelliJ version

(📚 Archive) (📈 Interdiff between v5 and v6)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/84/6/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

Remove Eclipse project files

735daca

As we are moving to IntelliJ, the Eclipse project files are no longer needed. Let's remove them.

CanIHasReview-bot added the s.ToReview label Aug 16, 2017

yamgent force-pushed the 83-migrate-intellij branch from 0c5723b to 1ae10bd Compare August 16, 2017 01:32

yamgent commented Aug 16, 2017

View reviewed changes

yamgent requested a review from damithc August 16, 2017 01:34

damithc suggested changes Aug 16, 2017

View reviewed changes

yamgent force-pushed the 83-migrate-intellij branch 2 times, most recently from 99ec3c1 to aa1379d Compare August 16, 2017 14:10

CanIHasReview-bot requested a review from damithc August 16, 2017 14:18

yamgent commented Aug 16, 2017

View reviewed changes

yamgent requested review from damithc and removed request for damithc August 17, 2017 08:22

damithc suggested changes Aug 17, 2017

View reviewed changes

yamgent force-pushed the 83-migrate-intellij branch 2 times, most recently from c8e8965 to 85c44c2 Compare August 19, 2017 03:43

CanIHasReview-bot requested a review from damithc August 19, 2017 03:46

damithc reviewed Aug 19, 2017

View reviewed changes

yamgent force-pushed the 83-migrate-intellij branch from 85c44c2 to 1df7fce Compare August 19, 2017 05:08

yamgent added 4 commits August 19, 2017 13:10

DeveloperGuide: Migrate setup instructions to IntelliJ

6c4f0c9

We are no longer using Eclipse as our development environment. The instructions in the developer guide are outdated. Let's migrate the setup instructions to IntelliJ.

UserGuide: Migrate instructions to IntelliJ

be3a3d4

We are no longer using Eclipse as our development environment. The instructions in the user guide are outdated. Let's migrate the instructions to IntelliJ.

LearningOutcomes: Update JavaFx tutorial to IntelliJ version

f52126c

yamgent force-pushed the 83-migrate-intellij branch from 1df7fce to f52126c Compare August 19, 2017 05:11

CanIHasReview-bot requested a review from damithc August 19, 2017 05:17

damithc approved these changes Aug 19, 2017

View reviewed changes

yamgent merged commit 0c99fd0 into se-edu:master Aug 19, 2017

yamgent deleted the 83-migrate-intellij branch August 19, 2017 16:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate project to use IntelliJ instead of Eclipse #83 #84

Migrate project to use IntelliJ instead of Eclipse #83 #84

yamgent commented Aug 16, 2017

mention-bot commented Aug 16, 2017

CanIHasReview-bot commented Aug 16, 2017

CanIHasReview-bot commented Aug 16, 2017

yamgent Aug 16, 2017

damithc Aug 16, 2017

damithc Aug 16, 2017

CanIHasReview-bot commented Aug 16, 2017

yamgent Aug 16, 2017

damithc Aug 16, 2017

yamgent Aug 17, 2017

yamgent commented Aug 16, 2017

damithc commented Aug 16, 2017

CanIHasReview-bot commented Aug 17, 2017

damithc left a comment

damithc Aug 17, 2017

damithc Aug 17, 2017

CanIHasReview-bot commented Aug 19, 2017

damithc Aug 19, 2017

CanIHasReview-bot commented Aug 19, 2017

Migrate project to use IntelliJ instead of Eclipse #83 #84

Migrate project to use IntelliJ instead of Eclipse #83 #84

Conversation

yamgent commented Aug 16, 2017

mention-bot commented Aug 16, 2017

CanIHasReview-bot commented Aug 16, 2017

v1

CanIHasReview-bot commented Aug 16, 2017

v2

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CanIHasReview-bot commented Aug 16, 2017

v3

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent commented Aug 16, 2017

damithc commented Aug 16, 2017

CanIHasReview-bot commented Aug 17, 2017

v4

damithc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CanIHasReview-bot commented Aug 19, 2017

v5

Choose a reason for hiding this comment

CanIHasReview-bot commented Aug 19, 2017

v6