mrcpappagallo / Reposecond

This is the 2nd repository for my test
0 stars 0 forks source link

Show admin UI on issues only when appropriate #3

Open mrcpappagallo opened 10 years ago

mrcpappagallo commented 10 years ago

This is re: #413. Before fields like labels and milestones were being shown to everyone when really they should only be shown to contributors. For some reason the diff went kind of crazy despite me not making a huge number of changes. If there's anything I can do to ease the review process definitely let me know

mrcpappagallo commented 10 years ago

I don't think you should introduce so much changes with this fix.

In issue_edit.xml you can only mark all collaborator related views as GONE and assign ids to title TextViews to mark them as VISIBLE later from code. I guess you have that huge diff in layout file because of formatting options in your editor.

In EditIssueActivity.java you can start checkCollaboratorStatus() from onCreate() and update labels/milestones/assignee section on success of this task.

Please, refactor the code and make the diff cleaner and I will merge your fix afterwards.

Also, consider rebasing and squashing your commits. Jake Wharton's rebase and squash script might help you with that.

mrcpappagallo commented 10 years ago

Gotcha. I initially had issue_edit.xml the way you described, but it seemed like I was doing assigning a whole lot of views to GONE when I could just wrap them all up as one. That being said, I can see where you're coming from, in that it adds a bunch of useless complexity to the layout.

Re: formatting. Are there any default formatting options that this project uses that are not the same as Android Studio default? It's my first time using AS, so I very well could have missed some sort of configuration for the project.

In any case, thanks for the feedback. I'll get to work on cleaning this up.

mrcpappagallo commented 10 years ago

I messed up my squashing (I'm working on another branch as well, so the script you posted wasn't usable) so my git history is a little bit messed up at the moment. I'll fix it (and do a little more diff cleanup) and post here once I'm done.

mrcpappagallo commented 10 years ago

@atermenji take a look now. History and diff cleanup are pretty much done. The one awkward thing is that because I copy/pasted the click listener code from onCreate to showCollaboratorOptions, adding the listener to the textWatcher appears as a diff. Is this avoidable?

mrcpappagallo commented 10 years ago

@maltzj thanks for this fix :+1:

I've made some small refactorings and merged this into master.

mrcpappagallo commented 10 years ago

Inserire commento test device

mrcpappagallo commented 10 years ago

provoletta

mrcpappagallo commented 10 years ago

Inserire commento sjjdudjsjsi sjisjshshsyys sjsisjshjsjshs shushshsuusjsjsj sgsushsggsgs sjjsjsjsjsjdjsjs

mrcpappagallo commented 10 years ago

Inserire commento 2

mrcpappagallo commented 10 years ago

commennnmmmmmj. ggfghb bhg