Skip to content

Conversation

@wesruv
Copy link
Contributor

@wesruv wesruv commented Sep 4, 2020

Related issue

What has changed and why

Developer experience, line length was set to 80 (the default) which felt small (to me)

Ready-for-merge Checklist

Check off items as they are completed. Feel free to delete items if they are not applicable.

  • Expected files: all files in this pull request are related to one request or issue (no stragglers or scope-creep).
  • Browser testing passed (regression testing only).
  • Repository compiles and tests pass.
  • Changelog updated (not needed for documentation updates).
  • Documentation (README.md, WHY.md, etc.) updated or added.

Merging

Please squash when merging and ensure your commit message uses conventional commit formatting.

Be sure to share your updates with the [email protected] mailing list!

@wesruv wesruv added the ready: code review Ready for code review! label Sep 4, 2020
Copy link
Contributor

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

@wesruv I don't see any updates to the prettier config: https://prettier.io/docs/en/configuration.html

@castastrophe
Copy link
Contributor

Really stoked about this update, thanks for tackling it. Check out the label details in the PR template for which labels need to be applied. Thanks!

@wesruv
Copy link
Contributor Author

wesruv commented Sep 4, 2020

See my first commit, it's the only change made in that one.

@castastrophe
Copy link
Contributor

Okay, thanks. I looked through the PR and was only seeing the prettier updates and missed the config file. Can you address the template and label feedback then when you’re able and I’ll branch test. Code looks good.

@wesruv
Copy link
Contributor Author

wesruv commented Sep 8, 2020

I'm not sure what this error means? I've never run into anything like this and don't know where to start:

 Run actions/labeler@v22s
##[error]Input required and not supplied: repo-token
Run actions/labeler@v2
##[error]Error: Input required and not supplied: repo-token
##[error]Input required and not supplied: repo-token

@wesruv
Copy link
Contributor Author

wesruv commented Sep 8, 2020

Noticed that we don't have trailing commas and that it's now the default in prettier 2.x, now noticing we are specifying: ^1.19.1

I would like to get trailing commas in arrays and objects and such, I think it reduces human error when developing, any reason why we couldn't try upgrading to 2.0?

@wesruv
Copy link
Contributor Author

wesruv commented Sep 8, 2020

... follow up question, should that be a different PR?

@castastrophe castastrophe added feature New feature or request ready: branch testing Test the component from a user-perspective. Try to break it! priority: low Severity level: 3 labels Sep 9, 2020
@castastrophe
Copy link
Contributor

@wesruv Updating to 2.0 sounds great but I like your suggestion to handle that in another PR, that keeps them pretty scoped which is nice. The labeler issue I can poke at - it has to do with our project's shared secret and how the workflow is utilizing it. Happy to show you some time if you're interested but I can handle it if not.

@castastrophe
Copy link
Contributor

@wesruv Ah it's because your PR is from a fork - the handles stored in the project can't be shared with a fork branch for security reasons. 🤔 Can you migrate the PR?

@wesruv
Copy link
Contributor Author

wesruv commented Sep 10, 2020

No problemo, it's here now: #1092

@wesruv wesruv closed this Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request priority: low Severity level: 3 ready: branch testing Test the component from a user-perspective. Try to break it! ready: code review Ready for code review!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants