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

App/folder structure #6

Merged
merged 1 commit into from
Sep 28, 2018
Merged

Conversation

bailer
Copy link
Contributor

@bailer bailer commented May 23, 2018

Changed the filename extensions of the react components to .jsx to make it easier for text editors to correctly select JSX syntax highlighting and to solve #2.

Also put the react components and util file into a separate folders to clean up the structure a bit.

Copy link
Contributor

@ninaolo ninaolo left a comment

Choose a reason for hiding this comment

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

Great that you found the solution for the GitHub highlighting issue 👍 🙂 Just some comments:

  1. Since this was a while ago (sorry for not addressing this PR) there's a conflict to be resolved.
  2. While you're at it, could you make two separate PRs for folder structure and syntax highlighting? And cherry pick the commits.
  3. Also, we'd prefer no broken commits, so please squash the broken commits you had there.

@mlenser mlenser force-pushed the client-structure-and-jsx branch 2 times, most recently from c2be00a to 6404569 Compare September 28, 2018 14:10
mlenser
mlenser previously approved these changes Sep 28, 2018
@mlenser
Copy link
Contributor

mlenser commented Sep 28, 2018

@bailer Nice to see you again Snakob!

@ninaolo ninaolo changed the title Client structure and jsx App/folder structure Sep 28, 2018
Copy link
Contributor

@ninaolo ninaolo left a comment

Choose a reason for hiding this comment

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

Nice!

@ninaolo ninaolo merged commit 77490a8 into tink-ab:master Sep 28, 2018
@ninaolo ninaolo mentioned this pull request Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants