-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BuildView refactoring #28
BuildView refactoring #28
Conversation
@DreierF Build is failing. Could you please fix it first? |
Oh. Sorry didn't see it. |
holder.mBuildView.setCommit(relatedCommit); | ||
holder.mBuildView.setState(relatedBuild); | ||
holder.mBuildView.setTitle( | ||
holder.mBuildView.getContext().getString(R.string.pull_request_number, request.getPullRequestNumber())); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DreierF shouldn't this class have BuildViewHolder
as ViewHolder? then we can remove BranchViewHolder
and item_pull_request.xml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides that looks good 👍 Looking forward to get merged it.
Fixed! Not sure how I could miss that one 😅 |
@DreierF It's failing now again. Could you fix it? |
it's crashing on openning any repo details.
|
Hi,
As proposed in the last pull request I have now refactored the BuildView, to be less coupled with Build and Branch. Therefore I introduced an interface IBuildState, that is implemented by Build and Branch.
The methods setBuildData and setPullRequestData have been inlined at call site to reduce the number of parameters.
Further all attributes that are shown in the BuildView have now a default visibility set to GONE and are set to VISIBLE when the attribute is actually set. This enables to also use the BuildView for the repository list.
Last but not least BuildItemViewHolder is now used for all lists, because they have identical behaviours.