Skip to content

Comments

Feature/two letter iso codes#825

Merged
pljones merged 1 commit intojamulussoftware:next-releasefrom
pljones:feature/two-letter-iso-codes
Sep 25, 2022
Merged

Feature/two letter iso codes#825
pljones merged 1 commit intojamulussoftware:next-releasefrom
pljones:feature/two-letter-iso-codes

Conversation

@pljones
Copy link
Contributor

@pljones pljones commented Sep 21, 2022 •
edited
Loading

Short description of changes
Update --serverinfo to mention two letter country codes.

Context: Fixes an issue? Related issues
FIXES: #822
Relates to jamulussoftware/jamulus#2841

Status of this Pull Request
Needs review. Contains external link.

What is missing until this pull request can be merged?
Built and test - looks okay to me:
http://jamuluswebsite.drealm.info/wiki/Running-a-Server

Does this need translation?

YES

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I'm sure that this Pull Request goes to the correct branch (I typed "YES" to translations and PR goes to next-release and I've checked that ... yes I have. Definitely.)

@pljones pljones added this to the Release 3.9.1 milestone Sep 21, 2022
@pljones pljones linked an issue Sep 21, 2022 that may be closed by this pull request
Document --serverinfo two-letter ISO country code support #822
Closed
ann0see
ann0see reviewed Sep 23, 2022
View reviewed changes
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Not sure if what removing the whitespace implies. But all in all, I think it's good.

@gilgongo @ignotus666 do you know anything about the whitespace?

@pljones
Copy link
Contributor Author

pljones commented Sep 24, 2022

Not sure if what removing the whitespace implies. But all in all, I think it's good.

@gilgongo @ignotus666 do you know anything about the whitespace?

I checked before and after builds:

http://jamuluswebsite.drealm.info/wiki/Software-Manual#command-line-options
https://jamulus.io/wiki/Software-Manual#command-line-options

The whitespace isn't meaningful. Ideally, for the website, it would be a table

short long description
-a --a-long-option what it does
--no-short-option what this one does

but that was a bit of a bigger change than I felt was appropriate here..! :)

hoffie
hoffie reviewed Sep 24, 2022
View reviewed changes
@pljones pljones force-pushed the feature/two-letter-iso-codes branch 2 times, most recently from 908d384 to 0e0fa56 Compare September 25, 2022 19:13
@pljones pljones force-pushed the feature/two-letter-iso-codes branch from 0e0fa56 to efc79c6 Compare September 25, 2022 19:15
hoffie
hoffie approved these changes Sep 25, 2022
View reviewed changes
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Thanks!

(Can't comment on the relevance of the whitespace changes)

@pljones pljones merged commit caf92ea into jamulussoftware:next-release Sep 25, 2022
@pljones pljones deleted the feature/two-letter-iso-codes branch September 25, 2022 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ann0see ann0see ann0see left review comments

@hoffie hoffie hoffie approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Release 3.9.1

Development

Successfully merging this pull request may close these issues.

Document --serverinfo two-letter ISO country code support

3 participants

@pljones @hoffie @ann0see