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

Fill all target_id for new moves #807

Merged
merged 2 commits into from
Jan 4, 2023
Merged

Fill all target_id for new moves #807

merged 2 commits into from
Jan 4, 2023

Conversation

giginet
Copy link
Contributor

@giginet giginet commented Jan 3, 2023 •
edited
Loading

I introduced new moves on #783.

However, all target_id was blank.
I filled all target_id columns for all new moves.

@giginet giginet mentioned this pull request Jan 3, 2023
Merged
giginet
giginet commented Jan 3, 2023
View reviewed changes
data/v2/csv/move_targets.csv
@@ -14,3 +14,4 @@ id,identifier
13,user-and-allies
14,all-pokemon
15,all-allies
16,fainting-pokemon
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Revival Blessing" is the first move to use for fainting Pokémon.
So I introduced a new category for the move.

data/v2/csv/moves.csv
879,electro-drift,9,12,100,5,100,0,10,3,,,,,
880,shed-tail,9,0,0,10,,0,7,1,,,,,
881,chilly-reception,9,14,0,10,,0,12,1,,,,,
882,tidy-up,9,0,0,10,,0,7,1,,,,,
Copy link
Contributor Author

@giginet giginet Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

"Tidy up" has various effects at once.
So it's difficult to set the existing target_id. I choose user for this move.
What do you think? Do you need a new move_target?

Copy link
Member

Choose a reason for hiding this comment

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

Nah, for now let's leave it like that :) Thanks!

@giginet giginet mentioned this pull request Jan 3, 2023
Open
@Naramsim Naramsim merged commit 5df381e into PokeAPI:master Jan 4, 2023
@pokeapi-machine-user
Copy link

A PokeAPI/api-data refresh has started. In 45 minutes the staging branch of PokeAPI/api-data will be pushed with the new generated data.

The staging branch will be deployed in our staging environment and you will be able to review the entire API.

A Pull Request (master<-staging) will be also created at PokeAPI/api-data and assigned to the PokeAPI Core team to be reviewed. If approved and merged new data will soon be available worldwide at pokeapi.co.

@Naramsim
Copy link
Member

Naramsim commented Jan 4, 2023

Hi @giginet , this PR was overwritten by #801, can you please open another PR fixing again the target_ids?

@giginet giginet deleted the move-targets branch January 5, 2023 07:58
@giginet
Copy link
Contributor Author

giginet commented Jan 5, 2023

Sure. I'll open another PR today.

@Naramsim Naramsim mentioned this pull request Jan 7, 2023
Merged
@giginet giginet mentioned this pull request Jan 8, 2023
Merged
@giginet
Copy link
Contributor Author

giginet commented Jan 8, 2023

Sorry for the delay for a few days 🙏

I opened new PR to fix conflicts.
#810

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Naramsim Naramsim Naramsim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@giginet @pokeapi-machine-user @Naramsim