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

Redux store #72

Merged
merged 10 commits into from
May 15, 2023
Merged

Redux store #72

merged 10 commits into from
May 15, 2023

Conversation

johncalesp
Copy link
Contributor

  • State management is handle by redux-store
  • Changed unit testing

@johncalesp johncalesp requested review from jimgao1 and michaelshin May 11, 2023 19:35

export default function Habitat({ habitatData }) {
export default function Habitat() {
Copy link

Choose a reason for hiding this comment

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

Function Habitat has 66 lines of code (exceeds 25 allowed). Consider refactoring.

["RTX4000", 20.2342],
];

const habitatData = {
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

@@ -32,11 +32,14 @@ import { loadJsonFiles } from "../utils/parsers";

import { useSelector } from "react-redux";

const ProviderPanel = ({ habitatData, cloudProviderURLs }) => {
const ProviderPanel = () => {
Copy link

Choose a reason for hiding this comment

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

Function ProviderPanel has 346 lines of code (exceeds 25 allowed). Consider refactoring.


const { ResizeObserver } = window;

const data = {
const habitatData = {
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

["RTX4000", 20.2342],
["demo", 1],
];
const noHabitatData = {
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

@michaelshin
Copy link
Contributor

Can you add more details on what you moved to Redux and what each reducer is responsible for?

* @returns The VSCode API handle
*/
function acquireApi() {
// if (typeof this.acquireApi.api == 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it

export default function Habitat({ habitatData }) {
export default function Habitat() {
const {analysisState} = useSelector((state) => state.analysisStateSliceReducer);
const habitatData = analysisState["habitat"];
const habitatIsDemo = habitatData.predictions?.find(
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be put in the redux state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I refactored that part

@michaelshin
Copy link
Contributor

I think this is a good start for us to use Redux. Did you have a way to separate what states should be in the store vs stored locally? If we can keep that consistent, then it would be better to maintain longterm

@michaelshin michaelshin self-requested a review May 15, 2023 14:50
export default function Habitat({ habitatData }) {
const habitatIsDemo = habitatData.predictions?.find(
(item) => item[0] === "demo" && item[1] === 1
export default function Habitat() {
Copy link

Choose a reason for hiding this comment

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

Function Habitat has 65 lines of code (exceeds 25 allowed). Consider refactoring.

@@ -21,11 +21,13 @@ import { energy_data, unitScale, numberFormat } from "../utils/utils";

import { useSelector } from "react-redux";

const EnergyConsumption = ({ energyData }) => {
const EnergyConsumption = () => {
Copy link

Choose a reason for hiding this comment

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

Function EnergyConsumption has 281 lines of code (exceeds 25 allowed). Consider refactoring.

@@ -32,12 +32,16 @@ import { loadJsonFiles } from "../utils/parsers";

import { useSelector } from "react-redux";

const ProviderPanel = ({ habitatData, cloudProviderURLs }) => {
const ProviderPanel = () => {
Copy link

Choose a reason for hiding this comment

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

Function ProviderPanel has 348 lines of code (exceeds 25 allowed). Consider refactoring.


const WelcomeScreen = ({ analysisState, vscodeApi }) => {
const WelcomeScreen = () => {
Copy link

Choose a reason for hiding this comment

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

Function WelcomeScreen has 38 lines of code (exceeds 25 allowed). Consider refactoring.

@@ -9,10 +9,12 @@ import { numberFormat } from "../utils/utils";

import { useSelector } from "react-redux";

const DeploymentTab = ({ habitatData,cloudProviderURLs }) => {
const DeploymentTab = () => {
Copy link

Choose a reason for hiding this comment

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

Function DeploymentTab has 31 lines of code (exceeds 25 allowed). Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented May 15, 2023

Code Climate has analyzed commit f8c482b and detected 11 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 5
Duplication 6

View more on Code Climate.

@johncalesp
Copy link
Contributor Author

I added more comments in redux/store/store.js to indicate the responsibilities of each slice.
As of now, redux is handling the major states (analysis, epochs, vscode connection), usually the ones that are shared between 2 or more components. That way each individual component can grab what it needs and transform or modify the data locally to render the information, but they can't modify the global state stored in redux.

@johncalesp johncalesp merged commit d0eee47 into main May 15, 2023
@johncalesp johncalesp deleted the redux-store branch May 15, 2023 15:49
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.

2 participants