Skip to content

Comments

RTK Eval#18

Open
JoeCarnahan42 wants to merge 23 commits intoprojectshft:mainfrom
JoeCarnahan42:main
Open

RTK Eval#18
JoeCarnahan42 wants to merge 23 commits intoprojectshft:mainfrom
JoeCarnahan42:main

Conversation

@JoeCarnahan42
Copy link

No description provided.

const SearchBar = () => {
const dispatch = useDispatch();

// Better validation?? //

Choose a reason for hiding this comment

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

you dont need // at the end

const schema = yup.object({
input: yup
.string()
.transform(function (value) {

Choose a reason for hiding this comment

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

why not es6?

);

const data = (weather, dataPoint) => {
const arr = [3, 12, 20, 28, 36];

Choose a reason for hiding this comment

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

what is this?

Choose a reason for hiding this comment

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

Don't be shy on naming consts for what they are, dont be too generic


const data = (weather, dataPoint) => {
const arr = [3, 12, 20, 28, 36];
return arr.map((number) => {

Choose a reason for hiding this comment

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

too generic argument name

});
};

const calcAvg = (numbers) => {

Choose a reason for hiding this comment

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

too generic argument name

if (!action.payload) {
return;
} else if (!action.payload[0]) {
alert(

Choose a reason for hiding this comment

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

alerting is too agresive, next time display the error to the user properly.

Co-authored-by: ronyrosenberg <39233986+ronyrosenberg@users.noreply.github.com>
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