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

Add CheckStyle #42

Merged
merged 6 commits into from
May 8, 2017
Merged

Add CheckStyle #42

merged 6 commits into from
May 8, 2017

Conversation

CoXier
Copy link
Contributor

@CoXier CoXier commented May 8, 2017

No description provided.

* @param uri Uri of the loaded image
*/
void loadThumbnail(Context context, int resize, Drawable placeholder, ImageView imageView, Uri uri);
void loadThumbnail(Context context, int resize, Drawable placeholder, ImageView iv, Uri uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this renaming your own favor or checkstyle's constraint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My own favor.

First I set max length 100 and I modified some file ,however later I found Matisse has many lines over 100 while it's very hard to cut down these line's length.So I set max length 120 thus as you see some changes are bad.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, such naming style can lead to sort of confusion, and we can wrap the line to avoid reaching the column limit.

Copy link
Contributor

Choose a reason for hiding this comment

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

@unixzii
@CoXier has reverted wrapping modifications. I think wrapping is not a problem, but you should wrap gracefully.

* @param imageView ImageView widget
* @param uri Uri of the loaded image
*/
void loadImage(Context context, int resizeX, int resizeY, ImageView imageView, Uri uri);
void loadImage(Context context, int rX, int rY, ImageView imageView, Uri uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think rX and rY‘s readability is not good.

}

@Override
public RecyclerView.ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
if (viewType == VIEW_TYPE_CAPTURE) {
View v = LayoutInflater.from(parent.getContext()).inflate(R.layout.photo_capture_item, parent, false);
View v = LayoutInflater.from(parent.getContext()).
inflate(R.layout.photo_capture_item, parent, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that if you set your IDE's max line length to be 120, this line should not wrap. The same below.

@@ -30,7 +30,7 @@

public class AlbumsSpinner {

private static final int sMaxShownCount = 6;
private static final int S_MAX_SHOWN_COUNT = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

This constant should be named MAX_SHOWN_COUNT

int baseX = (int) (canvas.getWidth() - mTextPaint.measureText(text)) / 2;
int baseY =
(int) (canvas.getHeight() - mTextPaint.descent() - mTextPaint.ascent()) / 2;
canvas.drawText(text, baseX, baseY, mTextPaint);
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong wrap under 120 max line length.

ArrayList<Item> selected = resultBundle.getParcelableArrayList(SelectedItemCollection.STATE_SELECTION);
int collectionType = resultBundle.getInt(SelectedItemCollection.STATE_COLLECTION_TYPE, SelectedItemCollection.COLLECTION_UNDEFINED);
ArrayList<Item> selected =
resultBundle.getParcelableArrayList(SelectedItemCollection.STATE_SELECTION);
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong wrap

@@ -206,7 +208,8 @@ private void updateBottomToolbar() {
public void onClick(View v) {
if (v.getId() == R.id.button_preview) {
Intent intent = new Intent(this, SelectedPreviewActivity.class);
intent.putExtra(BasePreviewActivity.EXTRA_DEFAULT_BUNDLE, mSelectedCollection.getDataWithBundle());
intent.putExtra(BasePreviewActivity.EXTRA_DEFAULT_BUNDLE,
mSelectedCollection.getDataWithBundle());
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong wrap

@@ -285,7 +288,8 @@ public void onMediaClick(Album album, Item item, int adapterPosition) {
Intent intent = new Intent(this, AlbumPreviewActivity.class);
intent.putExtra(AlbumPreviewActivity.EXTRA_ALBUM, album);
intent.putExtra(AlbumPreviewActivity.EXTRA_ITEM, item);
intent.putExtra(BasePreviewActivity.EXTRA_DEFAULT_BUNDLE, mSelectedCollection.getDataWithBundle());
intent.putExtra(BasePreviewActivity.EXTRA_DEFAULT_BUNDLE,
mSelectedCollection.getDataWithBundle());
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong wrap

@CoXier
Copy link
Contributor Author

CoXier commented May 8, 2017

In addition I have put CheckStyle in travis.

@CoXier CoXier mentioned this pull request May 8, 2017
@gejiaheng
Copy link
Contributor

@CoXier It is my fault that didn't mention max line length in the project.

@gejiaheng gejiaheng merged commit cd664e1 into zhihu:master May 8, 2017
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.

None yet

3 participants