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

Clean many labels of scripts of gyms #323

Merged
merged 1 commit into from
May 31, 2021
Merged

Clean many labels of scripts of gyms #323

merged 1 commit into from
May 31, 2021

Conversation

kqesar
Copy link
Contributor

@kqesar kqesar commented May 14, 2021

I have cleaned many labels and associated textPointer in the same ways that scripts/VermilionGym.asm and text/VermilionGym.asm

Related to issue #315

@kqesar kqesar changed the title refactor CeladonGymScript Refactor CeladonGymScript May 14, 2021
@kqesar kqesar changed the title Refactor CeladonGymScript Cleanup label from scripts Gyms: PewterGym, CeruleanGym, CeladonGym, FuchsiaGym and SaffronGym May 17, 2021
@kqesar kqesar force-pushed the master branch 2 times, most recently from f1e0344 to ba71807 Compare May 17, 2021 19:15
@kqesar kqesar changed the title Cleanup label from scripts Gyms: PewterGym, CeruleanGym, CeladonGym, FuchsiaGym and SaffronGym Cleanup label from scripts Gyms: PewterGym, CeruleanGym, CeladonGym, FuchsiaGym, SaffronGym and CinnabarGym May 17, 2021
@kqesar kqesar changed the title Cleanup label from scripts Gyms: PewterGym, CeruleanGym, CeladonGym, FuchsiaGym, SaffronGym and CinnabarGym Clean many labels of scripts of gyms May 17, 2021
scripts/CeladonGym.asm Outdated Show resolved Hide resolved
scripts/CeruleanGym.asm Outdated Show resolved Hide resolved
scripts/CeruleanGym.asm Outdated Show resolved Hide resolved
scripts/CeladonGym.asm Outdated Show resolved Hide resolved
scripts/CinnabarGym.asm Outdated Show resolved Hide resolved
scripts/CinnabarGym.asm Outdated Show resolved Hide resolved
scripts/CeruleanGym.asm Outdated Show resolved Hide resolved
@kqesar kqesar requested review from dannye and Rangi42 May 31, 2021 17:31
@Rangi42
Copy link
Member

Rangi42 commented May 31, 2021

@dannye Please squash this if you're merging it so it becomes one commit.

@kqesar
Copy link
Contributor Author

kqesar commented May 31, 2021

@Rangi42 I have squashed commit, now the PR have only one commit.

@dannye
Copy link
Member

dannye commented May 31, 2021

@dannye Please squash this if you're merging it so it becomes one commit.

@Rangi42 The commit log has always been a bit of a dogpile, so it doesn't seem super useful to start clean squashed merges now, but of course I agree with you in general.
Squashed or not, I at least like to merge rather than rebase to avoid invalidating the requester's branch.
I prefer it best when the requester pre-squashes (like KqesaR has already kindly done) but at the same time I don't like to always demand that the requester do this to avoid raising the barrier for entry, especially given that this isn't a particularly professional project. I'd like to hear your thoughts on my thoughts though.

@dannye
Copy link
Member

dannye commented May 31, 2021

@kqesar Thanks!

@dannye dannye merged commit c5bb400 into pret:master May 31, 2021
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.

3 participants