Skip to content

Comments

Remove remaining directory server mentions#3017

Merged
pljones merged 10 commits intojamulussoftware:mainfrom
pljones:bugfix/remove-remaining-directory-server-mentions
Apr 16, 2023
Merged

Remove remaining directory server mentions#3017
pljones merged 10 commits intojamulussoftware:mainfrom
pljones:bugfix/remove-remaining-directory-server-mentions

Conversation

@pljones
Copy link
Collaborator

@pljones pljones commented Feb 25, 2023 •
edited by ann0see
Loading

Short description of changes

The overall goal is that:

  • Directory is the Jamulus term for an application that supplies a list of Jamulus Servers on request (most often the Jamulus application running as --server --directoryaddress localhost (or -s -e 127.0.0.1)).
  • Directory Address is the IP address (IPv4 only, currently), possibly including port, used to contact the Directory.
  • "Directory server" is an invalid term in Jamulus vocabulary.

This removes remaining "directory server" mentions in the code base (based on grep -ni directory\ \*server src/*.*).

Apart from:

  • backwards compatibility for --directoryserver command line option
  • RPC doccomment

(Replaced: #2997 - this is more complete)

CHANGELOG: Server: Rename --directoryserver to --directoryaddress (and internal changes)

Context: Fixes an issue?

On going maintenance to avoid confusion in terminology.

Does this change need documentation? What needs to be documented and how?

Yes. --help output now says --directoryaddress plus a few other changes. Translations affected in places, too (I added some more server GUI ones).

Status of this Pull Request

It runs okay.

What is missing until this pull request can be merged?

  • Review.
  • Fixing generate_json_rpc_docs.py script (@hoffie ?)
  • Fixing clang_format...

Note: the Json RPC "directory_server" mention is in its external protocol, not just a comment, so needs consideration before just changing it. The other changes here affect nothing outside Jamulus.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@pljones
Copy link
Collaborator Author

pljones commented Feb 25, 2023 •
edited
Loading

Huh?
image
(Raised #3018)

And Github clang_format and Ubuntu seem to have got out of alignment on my machine again..!

@pljones
Copy link
Collaborator Author

pljones commented Feb 25, 2023

So

"--clang-format-executable" "/clang-format/clang-format10"

$ clang-format --version
Ubuntu clang-format version 14.0.0-1ubuntu1

I guess I need to find clang-format-10 again...

@pljones
Copy link
Collaborator Author

pljones commented Feb 25, 2023 •
edited
Loading

Well, so clang-format-10 is no longer available and installing 11 made no difference locally.

$ sudo apt install clang-format<TAB><TAB>
clang-format     clang-format-11  clang-format-12  clang-format-13  clang-format-14  clang-format-15

(Raised #3019)

This was referenced Feb 25, 2023
Closed
Closed
ann0see
ann0see reviewed Feb 25, 2023
View reviewed changes
@ann0see
Copy link
Member

ann0see commented Feb 25, 2023

Would --directory instead of --directoryaddress be confusing?

@pljones
Copy link
Collaborator Author

pljones commented Feb 25, 2023

Would --directory instead of --directoryaddress be confusing?

I had --directory until this afternoon. But it's the address (in Internet Address including Port terms) of the Directory, not the Directory itself. See "The overall goal is that:" in the PR description.

@pljones pljones force-pushed the bugfix/remove-remaining-directory-server-mentions branch 2 times, most recently from 592a085 to f047a8b Compare February 25, 2023 18:37
pljones
pljones commented Feb 25, 2023
View reviewed changes
pljones
pljones commented Feb 25, 2023
View reviewed changes
@pljones pljones force-pushed the bugfix/remove-remaining-directory-server-mentions branch from f047a8b to 80452df Compare March 26, 2023 15:16
ann0see
ann0see reviewed Mar 26, 2023
View reviewed changes
ann0see
ann0see reviewed Mar 26, 2023
View reviewed changes
pljones added a commit to jamulussoftware/jamuluswebsite that referenced this pull request Mar 26, 2023
@pljones
Update Style-and-Tone.md
add72ea 
Per jamulussoftware/jamulus#3017 (comment) - proposed change.
This was referenced Mar 26, 2023
Closed
Merged
@pljones pljones force-pushed the bugfix/remove-remaining-directory-server-mentions branch 2 times, most recently from 7394039 to f6b99a4 Compare March 28, 2023 20:28
@pljones
Copy link
Collaborator Author

pljones commented Apr 4, 2023

OK, I don't think there's anything outstanding here now?

ann0see
ann0see reviewed Apr 4, 2023
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.

See comment.

@pljones pljones mentioned this pull request Apr 5, 2023
Merged
5 tasks
@pljones pljones force-pushed the bugfix/remove-remaining-directory-server-mentions branch from f6b99a4 to 8f09aff Compare April 6, 2023 17:42
@pljones pljones self-assigned this Apr 9, 2023
@pljones pljones added refactoring Non-behavioural changes, Code cleanup needs documentation PRs requiring documentation changes or additions labels Apr 9, 2023
@pljones pljones added this to the Release 3.10.0 milestone Apr 9, 2023
ann0see
ann0see reviewed Apr 12, 2023
View reviewed changes
ann0see
ann0see reviewed Apr 12, 2023
View reviewed changes
ann0see
ann0see approved these changes Apr 12, 2023
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.

I think by sight reviewing this looks ok.

@pljones pljones force-pushed the bugfix/remove-remaining-directory-server-mentions branch from 8f09aff to b1f9602 Compare April 13, 2023 17:22
@pljones pljones merged commit b6314d0 into jamulussoftware:main Apr 16, 2023
@pljones pljones deleted the bugfix/remove-remaining-directory-server-mentions branch April 16, 2023 15:09
@pljones pljones mentioned this pull request Apr 17, 2023
Closed
@pljones
Copy link
Collaborator Author

pljones commented Apr 17, 2023

Raised jamulussoftware/jamuluswebsite#919 for the documentation change.

@pljones
Copy link
Collaborator Author

pljones commented Apr 18, 2023 •
edited
Loading

https://github.com/jamulussoftware/jamulus/actions/runs/4713980244/jobs/8379530170 is still failing on main (but didn't on the branch, which was up to date with main...). Anyone understand why?

Raised #3049

This was referenced May 31, 2023
Closed
Merged
@ann0see
Copy link
Member

ann0see commented Jul 24, 2023

@pljones I think we can drop the needs documentation flag here too.

@pljones pljones removed the needs documentation PRs requiring documentation changes or additions label Jul 25, 2023
@ann0see ann0see mentioned this pull request Jul 29, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ann0see ann0see ann0see approved these changes

Assignees

@pljones pljones

Labels

refactoring Non-behavioural changes, Code cleanup

Projects

Tracking
Status: Done

Milestone

Release 3.10.0

Development

Successfully merging this pull request may close these issues.

2 participants

@pljones @ann0see