InstructorCoursesPage: Put 'archive' before 'delete'

[damithc]

From dam...@gmail.com on March 01, 2014 11:28:38

Current: Enroll View Edit Delete Archive Proposed: Enroll View Edit Archive Delete

Original issue: http://code.google.com/p/teammatespes/issues/detail?id=1654

[damithc]

From dam...@gmail.com on March 01, 2014 23:29:07

Nalin, have a go at this one.

Owner: nalin.ch...@gmail.com

[damithc]

From nalin.ch...@gmail.com on March 02, 2014 11:39:55

Dear sir,

I have made the changes and pushed the modified code at my server side clone of teammates[1] I also have deployed the modified project at my staging server[2] The 'diff file' can be found here[3] Since I am new to community, please indicate if this is the correct way to submit "review requests"(for bugs I may find and patches I may submit in future) :)

Attachment contains a screenshot of modified UI on 'InstructorCourse' Page

Testing: patch tested at development server(localhost), staging server on Linux Mint 12 (Firefox and Chromium)

Regards, Nalin Chhibber www.nalinchhibber.com

[1] https://code.google.com/r/nalinchhibber-teammates/ [2] https://teammates-nalin.appspot.com [3] https://code.google.com/r/nalinchhibber-teammates/source/diff?spec=svnc9a382168553d4d069226597c3e5fa7fcc403ec2&r=c9a382168553d4d069226597c3e5fa7fcc403ec2&format=side&path=/src/main/webapp/jsp/instructorCourses.jsp

Attachment: Screenshot.png

[damithc]

From nalin.ch...@gmail.com on March 02, 2014 13:40:41

I just went through the proper channel. It was clearly mentioned in DevMan. I just skipped that step(my bad) [uploaded diff(s) at 'codereview' using upload.py] https://codereview.appspot.com/70230047/ Hope its right this time :)

Status: ReadyForReview

[damithc]

From dam...@gmail.com on March 02, 2014 19:21:13

Cc: arnold.k...@gmail.com
Labels: Reviewer-Arnold

[damithc]

From arnold.k...@gmail.com on March 02, 2014 22:14:04

Hi Nalin, did you run the tests before uploading the patch? Some tests broke due to the changes you made. Please ensure 'Dev Green' before you upload a patch next time :)

Status: ChangesRequested

[damithc]

From nalin.ch...@gmail.com on March 04, 2014 02:51:37

https://codereview.appspot.com/69860052/

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 04, 2014 08:16:05

Hi Nalin, use the -i flag to upload your patch to the same issue in the review site as the first patch so I can see the diffs between the two patches. See DevMan for details :)

Status: ChangesRequested

[damithc]

From nalin.ch...@gmail.com on March 04, 2014 08:24:45

https://codereview.appspot.com/70230047/

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 04, 2014 18:31:23

Nalin, did you merge the latest changes to your branch before uploading the patch? I can't seem to apply the patch as the diffs doesn't match the latest version.

[damithc]

From nalin.ch...@gmail.com on March 04, 2014 23:39:52

Hello Arnold,

Apologies for skipping that. I have pushed the changes to my repository. I also have added you as an 'instructor' at my staging server. Please have a look again :)

Repository: https://code.google.com/r/nalinchhibber-teammates/ Staging Server: https://teammates-nalin.appspot.com/

[damithc]

From arnold.k...@gmail.com on March 05, 2014 08:00:43

Sorry, I think I wasn't clear. When I said "merge the latest changes to your branch" I mean doing a hg pull to get the latest changes from the repo, and then merging that with your changes in your local branch. Then you can reupload the patch so I can merge it :)

Status: ChangesRequested

[damithc]

From nalin.ch...@gmail.com on March 05, 2014 10:02:30

Hi Arnold

Ah, I guess I got you this time :) I finally set remote to 'master repo' and 'pulled' all changes(probably new fixes by fellow participants) Merged my changes and pushed everything to my personal repo :)

This should work now. Please check :)

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 05, 2014 18:40:02

Nalin, please read point 7 of the Fixing Issues section in DevMan carefully and follow the instructions step by step https://teammatesv4.appspot.com/dev/devman.html#process-fixingissues Firstly, you need to commit the changes in your own branch, not on default branch. It is ok to do this locally on the master repo as you won't be pushing the commits; the branch will only be visible to you.

Secondly, you need to use upload.py to upload a patch to the code review site so I can download the changes you made, not push it to your clone repo.

Status: ChangesRequested

[damithc]

From nalin.ch...@gmail.com on March 05, 2014 21:15:05

Patch 2 is the correct one. I am rolling back unnecessary changes (as in patch 3, that seems an upload.py issue) https://codereview.appspot.com/70230047/#ps20001

[damithc]

From arnold.k...@gmail.com on March 05, 2014 23:11:16

But isn't patch 2 the one from a couple of days ago which is not merged yet and can't be applied?

[damithc]

From nalin.ch...@gmail.com on March 06, 2014 09:19:43

Yes indeed

But I have a doubt, is patch #3 correct ? It shows some changes(though harmless) in .project and src/main/webapp/WEB-INF/appengine-web.template.xml. Also, upload.py got uploaded as a diff automatically which is strange.

I didnt essentially changed any code after comment #10. But Yes, I did sync'd my repo with master/committer repository after that. After your suggestions at comment #11 and #13, I did hg pull https://code.google.com/p/teammatespes/ hg merge default hg commit -m "commit message"

After that, I pushed changes to my own clone(nalinchhibber-teammates) and finally uploaded diff (patch 3) again. The primary issue(InstructorCoursesPage: Put 'archive' before 'delete') seem to be fixed however. Where should I move from here? Suggestions please :) :P

[damithc]

From arnold.k...@gmail.com on March 06, 2014 18:37:55

Yes, patch 3 is correct, I am able to apply it fine.

Changes to .project and .settings/com.google.appengine.eclipse.core.prefs should be reverted as those are automatic changes made by eclipse when you first open the project.

Changes to src/main/webapp/WEB-INF/appengine-web.template.xml should be reverted as this is the generic template file. Your specific setting should be placed in appengine-web.xml instead

Upload.py is uploaded as its mode is changed (you probably did a +x?) and is normal.

Just revert the ones I mentioned above and we're good to go :)

Status: ChangesRequested

[damithc]

From nalin.ch...@gmail.com on March 07, 2014 13:25:40

Hey,

I have reverted the unnecessary file changes to my clone :) (.project , .settings/com.google.appengine.eclipse.core.prefs and src/main/webapp/WEB-INF/appengine-web.template.xml as same as they were earlier) https://code.google.com/r/nalinchhibber-teammates/source/detail?r=2f5383aa76e5fbeddd1c40367000a6e90845860e Issue 1654 seem to be resolved without any undesired effect. This should work now, right? :)

[damithc]

From nalin.ch...@gmail.com on March 08, 2014 02:02:31

https://codereview.appspot.com/70230047/

Status: ReadyForReview

[damithc]

From arnold.k...@gmail.com on March 09, 2014 20:08:15

Um, if you see the diff at the review site, the "base" seem to already have the archive and delete swapped (which is not true in the current public default branch). Did you accidentally comitted some of your changes in the default branch? https://codereview.appspot.com/70230047/diff/60001/src/main/webapp/jsp/instructorCourses.jsp

Status: ChangesRequested

[damithc]

From nalin.ch...@gmail.com on March 02, 2014 11:39:55

Dear sir,

I have made the changes and pushed the modified code at my server side clone of teammates[1] I also have deployed the modified project at my staging server[2] The 'diff file' can be found here[3] Since I am new to community, please indicate if this is the correct way to submit "review requests"(for bugs I may find and patches I may submit in future) :)

Attachment contains a screenshot of modified UI on 'InstructorCourse' Page

Testing: patch tested at development server(localhost), staging server on Linux Mint 12 (Firefox and Chromium)

Regards, Nalin Chhibber www.nalinchhibber.com

[1] https://code.google.com/r/nalinchhibber-teammates/ [2] https://teammates-nalin.appspot.com [3] https://code.google.com/r/nalinchhibber-teammates/source/diff?spec=svnc9a382168553d4d069226597c3e5fa7fcc403ec2&r=c9a382168553d4d069226597c3e5fa7fcc403ec2&format=side&path=/src/main/webapp/jsp/instructorCourses.jsp