Skip to content
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

Update coding guidelines for using View Bindings #6

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

maskaravivek
Copy link
Member

Updating the doc based on the discussion in commons-app/apps-android-commons#3530

Copy link
Member

@domdomegg domdomegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. As we could edit the wiki before, feel free to merge future pull requests without review for now - we can adjust that if it becomes a problem in future.

@macgills macgills merged commit 5d60492 into commons-app:master Mar 18, 2020
Copy link
Member

@sivaraam sivaraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the late comment. Just wanted to share my concern regarding a possible contradiction.

```
### Butterknife

Contributors have the option of using [butterknife](https://github.com/JakeWharton/butterknife) for their view bindings. We recommend doing so if you can.
Copy link
Member

@sivaraam sivaraam Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this sentence kind of contradict the following sentence found above?

We have decided to move away from Butterknife and gradually replace its usage with Kotlin Android Extensions ViewBinding.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have the availability to make a PR to delete the recommendation?

Copy link
Member

@sivaraam sivaraam Mar 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Just so I understand correctly, do you mean this:

Suggested change
Contributors have the option of using [butterknife](https://github.com/JakeWharton/butterknife) for their view bindings. We recommend doing so if you can.
Contributors have the option of using [butterknife](https://github.com/JakeWharton/butterknife) for their view bindings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I've created #9 with the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants