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 pages company aa1 to implement 1 #41

Merged
merged 7 commits into from
Jun 6, 2020
Merged

Conversation

pinterid
Copy link
Member

@pinterid pinterid commented Jun 6, 2020

The companypage component has been
added. The page is not routed yet.

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change?
  • Have you tested your changes with successful results?

Type of Changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation (non-breaking change which adds documentation)
  • Breaking change (fix or feature that would cause existing functionality to change)

What is the current behavior? (link to any open issues here)

What is the new behavior (if this is a feature change)?

Other information:

  • Ref: 🐍

The companypage component has been
added. The page is not routed yet.
@pinterid pinterid added the enhancement New feature or request label Jun 6, 2020
@pinterid pinterid added this to the SNEK Version 0.1.0 milestone Jun 6, 2020
@pinterid pinterid self-assigned this Jun 6, 2020
The code quality has been improved.
react-chartjs-2 package has been added.
Copy link
Member

@schettn schettn left a comment

Choose a reason for hiding this comment

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

I left some suggestions.

src/components/pages/CompanyPage/index.jsx Show resolved Hide resolved
All suggestions of @schettn are applied now.
@schettn schettn self-requested a review June 6, 2020 18:09
Copy link
Member

@schettn schettn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member Author

@pinterid pinterid left a comment

Choose a reason for hiding this comment

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

Some changes have to be made.

src/components/pages/CompanyPage/index.jsx Outdated Show resolved Hide resolved
src/components/pages/CompanyPage/index.jsx Show resolved Hide resolved
Suggestions of @pinterid are applied now.

Co-authored-by: Pinterics David <[email protected]>
Copy link
Member

@kleberbaum kleberbaum left a comment

Choose a reason for hiding this comment

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

Should be ok to merge after a few changes.

src/components/pages/CompanyPage/index.jsx Show resolved Hide resolved
@schettn schettn requested a review from kleberbaum June 6, 2020 18:24
Copy link
Member

@kleberbaum kleberbaum left a comment

Choose a reason for hiding this comment

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

Should be ok to merge.

The code quality was improved due to a request by Codacy.
The code quality was improved due to a request by Codacy.
@schettn schettn merged commit 31c116e into implement-1 Jun 6, 2020
@pinterid pinterid mentioned this pull request Jun 7, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants