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

Fix #8162: Improve Error Message When Converting Town Owned Road #8247

Merged
merged 2 commits into from Jul 10, 2020

Conversation

@TrevorShelton
Copy link
Contributor

@TrevorShelton TrevorShelton commented Jun 28, 2020

This changes the error when you attempt to convert a road owned by a town to another road, specifying that it's owned by the town rather than simply being incompatible. As the original poster of the issue pointed out, these seemed to be the only occurences of the incompatible road string, so now it's unused, but they would be left untouched in case of future use or since changing it to a different error would do the work of removing it then. If requested, it likely wouldn't be too difficult to remove the string entirely.

This changes the error when you attempt to convert a road owned by a town to another road, specifying that it's owned by the town rather than simply being incompatible. As the original poster of the issue pointed out, these seemed to be the only occurences of the incompatible road string, so now it's unused, but they would be left untouched in case of future use or since changing it to a different error would do the work of removing it then. If requested, it likely wouldn't be too difficult to remove the string entirely.
@LordAro
Copy link
Member

@LordAro LordAro commented Jun 28, 2020

Unused strings should be removed (in a separate commit, to keep changes clean)

I'd consider a different (title) message for the commit as well - the abbreviations don't look great. "Fix #8162: Improve error message when converting town owned road", perhaps.

@TrevorShelton TrevorShelton changed the title Change #8162: Conversion of Town Road, Impr. Error Fix #8162: Improve Error Message When Converting Town Owned Road Jun 28, 2020
@TrevorShelton
Copy link
Contributor Author

@TrevorShelton TrevorShelton commented Jun 28, 2020

Alright, changed the title to that.

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Jun 28, 2020

Please just remove the old string from all the language files at once, instead of one file/one commit at a time.

@TrevorShelton
Copy link
Contributor Author

@TrevorShelton TrevorShelton commented Jun 28, 2020

Oh, sorry, didn't come back to read this until I was done erasing it from all of the files. Is there a way to merge these or do I have to go back and use something else to do the erasure? (Sorry, fairly new to github commits.)

@LordAro
Copy link
Member

@LordAro LordAro commented Jun 28, 2020

Yes... Using the GitHub web interface isn't the greatest normally, and especially not when you have to edit multiple files. Get a local checkout, and edit the files there.

https://guides.github.com/introduction/git-handbook/
https://help.github.com/en/github/using-git/about-git-rebase

@LordAro
LordAro approved these changes Jul 2, 2020
@LordAro LordAro merged commit c39d62c into OpenTTD:master Jul 10, 2020
8 checks passed
8 checks passed
Commit checker
Details
OpenTTD CI Build #20200628.39 succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@glx22 glx22 added backported and removed backport requested labels Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.