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.

Sign up for GitHub

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

Jump to bottom

chore(gatsby): Migrate nodes reducer and last-action reducer to TypeScript #23771

Merged
merged 17 commits into from
May 8, 2020
Merged

chore(gatsby): Migrate nodes reducer and last-action reducer to TypeScript #23771

merged 17 commits into from
May 8, 2020

Conversation

alisson-suzigan
Copy link
Contributor

@alisson-suzigan alisson-suzigan commented May 5, 2020 •
edited
Loading

Description

This PR converts the following files to TS:

  • packages/gatsby/src/redux/reducers/nodes.js
  • packages/gatsby/src/redux/tests/nodes.js
  • packages/gatsby/src/redux/reducers/last-action.js
  • packages/gatsby/src/redux/tests/snapshots/nodes.js.snap

Add new interfaces on:

  • packages/gatsby/src/redux/types.ts

And update related imports on:

  • packages/gatsby/src/redux/reducers/index.js

Related Issue

Related to #21995

@alisson-suzigan alisson-suzigan requested a review from a team as a code owner May 5, 2020 01:57
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 5, 2020
LekoArts
LekoArts suggested changes May 5, 2020
View reviewed changes
Copy link
Contributor

@LekoArts LekoArts left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the PR! Left some comments. Can you also resolve the merge conflicts?

packages/gatsby/src/redux/reducers/index.js Outdated Show resolved Hide resolved
packages/gatsby/src/redux/reducers/last-action.ts Show resolved Hide resolved
@LekoArts LekoArts added type: TypeScript migration and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 5, 2020
@alisson-suzigan alisson-suzigan changed the title TS migration: nodes reducer and last-action reducer chore(gatsby): Migrate nodes reducer and last-action reducer to TypeScript May 5, 2020
@alisson-suzigan
Copy link
Contributor Author

@LekoArts, in the packages/gatsby/src/redux/reducers/index.js file we have 2 blocks of code (the cases loki on lines 22 and 39) that will never be reached.
Could I remove those lines?

@alisson-suzigan alisson-suzigan requested review from a team as code owners May 6, 2020 02:27
@LekoArts LekoArts removed request for a team May 6, 2020 11:56
@LekoArts
Copy link
Contributor

LekoArts commented May 6, 2020

Please don't request reviews from parties that are not involved. Thanks!

@LekoArts
Copy link
Contributor

LekoArts commented May 6, 2020 •
edited
Loading

in the packages/gatsby/src/redux/reducers/index.js file we have 2 blocks of code (the cases loki on lines 22 and 39) that will never be reached.
Could I remove those lines?

Since this PR is about converting the files I don't think this is the scope of this PR and you can keep it as-is for now :)

@alisson-suzigan
Copy link
Contributor Author

Thanks for the PR! Left some comments. Can you also resolve the merge conflicts?

I tried to resolve the conflicts yesterday, but I couldn't.
I was going to continue the work today, but I see that you've fixed it. 👏 🚀

@alisson-suzigan
Copy link
Contributor Author

@LekoArts, I ran the code locally with the most recent changes and everything worked fine.
Do you think it is required any other adjustments in this PR?

@blainekasten blainekasten mentioned this pull request May 7, 2020
Closed
@alisson-suzigan alisson-suzigan self-assigned this May 7, 2020
LekoArts
LekoArts approved these changes May 8, 2020
View reviewed changes
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@LekoArts LekoArts merged commit 197614e into gatsbyjs:master May 8, 2020
@alisson-suzigan alisson-suzigan deleted the ts/reducers-nodes branch May 8, 2020 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LekoArts LekoArts LekoArts approved these changes

Assignees

@alisson-suzigan alisson-suzigan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alisson-suzigan @LekoArts