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

Replace outdated Mercurial stuff, update README #7

Merged
merged 10 commits into from Mar 15, 2021
Merged

Conversation

Wuzzy2
Copy link
Contributor

@Wuzzy2 Wuzzy2 commented Feb 9, 2021

Fixes #5. Fixes #6.

This PR updates the README and fixes many outdated instructions, links and references to Mercurial-related stuff to replace it with Git-related stuff.

This PR also updates the Makefiles and .hgignore.

LordAro
LordAro previously approved these changes Feb 9, 2021
Copy link
Member

@LordAro LordAro left a comment

Seems reasonable

@Wuzzy2
Copy link
Contributor Author

@Wuzzy2 Wuzzy2 commented Feb 9, 2021

Please test if it still builds for you without error! That's really important. :D

@Wuzzy2 Wuzzy2 mentioned this pull request Mar 2, 2021
@orudge
Copy link
Contributor

@orudge orudge commented Mar 2, 2021

I haven't tested this, but given there were issues with the equivalent changeset on OpenSFX, I suspect we'll basically need to copy those same changes over. Also the appropriate GitHub Actions infrastructure.

@Wuzzy2
Copy link
Contributor Author

@Wuzzy2 Wuzzy2 commented Mar 15, 2021

I've looked at the script again, and and OpenSFX, and made the following changes:

  • Generate proper bundle name with version number attached (based on Git tag), fix version number in README
  • Add findversion.sh
  • Fixed unix2dos detection (identical bug in OpenSFX)

As for GitHub Actions, I suggest to do this stuff separately (different PR).

@planetmaker
Copy link
Contributor

@planetmaker planetmaker commented Mar 15, 2021

I just pulled the branch and tried to build it, yet it failed.
~/ottd/OpenMSX] kill-hg(+1/-0) 2 ± LC_ALL=C make
Makefile:161: *** unterminated call to function 'shell': missing ')'. Stop.

The problem here is case:
shell is not the same as SHELL

you should use consequently SHELL
(that also fits the style guide that variables are UPPERCASE)

@LordAro
Copy link
Member

@LordAro LordAro commented Mar 15, 2021

No? Well, kinda, but not quite. $(shell ...) is the function, SHELL is the variable that sets the shell to use. Is /bin/bash not valid on your system, I wonder?
(This also seems very similar to Eddi's OpenTTD/grfcodec#11 )

https://www.gnu.org/software/make/manual/html_node/Shell-Function.html
https://www.gnu.org/software/make/manual/html_node/Choosing-the-Shell.html

@Wuzzy2
Copy link
Contributor Author

@Wuzzy2 Wuzzy2 commented Mar 15, 2021

Well, it works for me. *shrug*

@orudge
Copy link
Contributor

@orudge orudge commented Mar 15, 2021

If in doubt, just copy what already works (OpenSFX). :)

The whole lot should be squashed into a single commit, but that can be done at merge time. I'll have a look at the rest of the changes.

docs/readme.ptxt Outdated Show resolved Hide resolved
docs/readme.ptxt Outdated Show resolved Hide resolved
docs/readme.ptxt Outdated Show resolved Hide resolved
@Wuzzy2
Copy link
Contributor Author

@Wuzzy2 Wuzzy2 commented Mar 15, 2021

I agree all the commits should be squashed at the end, but not now, before the review/approval is done.

orudge
orudge approved these changes Mar 15, 2021
@orudge orudge merged commit 2aa62cd into OpenTTD:master Mar 15, 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.

4 participants