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

test commit #20

Merged
merged 11 commits into from
May 8, 2024
Merged

test commit #20

merged 11 commits into from
May 8, 2024

Conversation

alex-byteport
Copy link
Collaborator

No description provided.

@brenzi
Copy link
Contributor

brenzi commented May 7, 2024

Thank you @alex-byteport. The only thing I see here is changes in formatting. I think we need to find an agreement on this because I don't want to see formatting changes in PRs that do other stuff too. reviewing is too painful if 90% is formatting changes. I can apply your formatting style in my IDE as well, but I certainly only want to do this change once

May I ask you to always use the PR description field to briefly describe what this PR changes?

@brenzi
Copy link
Contributor

brenzi commented May 7, 2024

I introduced linting and autoformatting in #21. Please follow these formatting rules from now onwards. you can just run prettier . -write to apply autoformatting

@brenzi
Copy link
Contributor

brenzi commented May 7, 2024

If this PR has more than just code formatting, please resolve conflicts and autoformat. otherwise, please close.

@brenzi
Copy link
Contributor

brenzi commented May 8, 2024

fixed functionality and took your example to improve the design in other places too @alex-byteport. Thanks for the nice inputs!

Problems with this PR were:

  • your IDE seems to have its own head about code formatting. I now introduced formatting checker in CI. This way we should always be able to reproduce the desired formatting with prettier . --write
  • I guess you used a different package manager than yarn. Please do not commit those lockfiles.
  • It seems you upgraded dependencies, which bricked the main functionality. I'd ask you to always run the entire flow as a test to see if everything still works

@brenzi brenzi merged commit 7cb5d1b into master May 8, 2024
2 checks passed
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