Skip to content

Lint#3437

Merged
fregante merged 21 commits intorefined-github:masterfrom
yakov116:lint
Aug 10, 2020
Merged

Lint#3437
fregante merged 21 commits intorefined-github:masterfrom
yakov116:lint

Conversation

@yakov116
Copy link
Member

@yakov116 yakov116 commented Aug 6, 2020

Due to popular demand I have opened another lint PR. #3412 (comment)

I changed the select.all's to (almost) always be an array. Its easier to read IMHO and gives the ability to add comments.

const cancelButton = select<HTMLButtonElement>([
'.js-hide-inline-comment-form',
'.js-comment-cancel-button'
], field.form!);
Copy link
Member

@fregante fregante Aug 6, 2020

Choose a reason for hiding this comment

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

The only reason to do this is to add comments. If comments aren’t there or aren’t necessary, there’s no need to do this.

const comments = select.all([
':not(.js-new-comment-form):not([id^="pullrequestreview"]) > .timeline-comment:not(.rgh-time-machine-links)',
'.review-comment > .previewable-edit:not(.is-pending):not(.rgh-time-machine-links)'
]);
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be commented? Can you add comments if they help?

Copy link
Member Author

Choose a reason for hiding this comment

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

Comments wont help much. I think the classes are self explanatory

Copy link
Member

Choose a reason for hiding this comment

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

Good. I'll revert

@fregante fregante added the meta Related to Refined GitHub itself label Aug 6, 2020
@yakov116 yakov116 marked this pull request as draft August 7, 2020 23:05
@fregante
Copy link
Member

fregante commented Aug 9, 2020

Can you also fix the capitalization here? sindresorhus/onetime#12

Copy link
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

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

Looks good so far! 🧶

@yakov116
Copy link
Member Author

@fregante you can merge. I am going on vacation for a week starting tomorrow. and will have very limited access

@yakov116 yakov116 marked this pull request as ready for review August 10, 2020 15:28
@fregante
Copy link
Member

Enjoy!

@fregante fregante merged commit e93ffb7 into refined-github:master Aug 10, 2020
@yakov116
Copy link
Member Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Related to Refined GitHub itself

Development

Successfully merging this pull request may close these issues.

2 participants