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 new icons: 2x2x2 OH, Mirror Blocks Blindfolded, 4x4x4 With Feet #117

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

dmint789
Copy link
Contributor

No description provided.

Copy link
Member

@jfly jfly left a comment

Choose a reason for hiding this comment

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

The icons LGTM! But @lgarron might have opinions on naming.

@dmint789
Copy link
Contributor Author

@jfly we've actually combined forces with the unofficial events stuff, and the naming is exactly the same as on our cubingcontests.com website, so you can just go ahead with the merge. We'll also likely be moving this project to the cubing organization in the near future.

@jfly jfly merged commit 5f47f67 into cubing:main Oct 30, 2023
1 check passed
Copy link
Member

@lgarron lgarron left a comment

Choose a reason for hiding this comment

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

These LGTM, although I'd still like a chance to review new icons in the future.

@jfly
Copy link
Member

jfly commented Oct 30, 2023

I'd still like a chance to review new icons in the future

Let's have a separate conversation about that someday? I'm not immediately thrilled about that: I personally have never worked on project that requires more than 1 person (besides the contributor) to approve every change.

@dmint789
Copy link
Contributor Author

I agree, I feel like requiring multiple reviews for such a small project is too much. Maybe we could just give 24 hours for people to leave their thoughts in the future.

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.

3 participants