Skip to content

Comments

RPC: remove remaining directory server mentions#3048

Merged
pljones merged 2 commits intojamulussoftware:mainfrom
pljones:bugfix/rpc-remaining-directory-server-mentions
Apr 17, 2023
Merged

RPC: remove remaining directory server mentions#3048
pljones merged 2 commits intojamulussoftware:mainfrom
pljones:bugfix/rpc-remaining-directory-server-mentions

Conversation

@pljones
Copy link
Collaborator

@pljones pljones commented Apr 16, 2023 •
edited by ann0see
Loading

Short description of changes

CHANGELOG: RPC: Rename directoryServer to directory in jamulusserver/getServerProfile response

Context: Fixes an issue?

RPC changes separated from main change due to RPC doc issues that got resolved.

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

RPC doc changes in line with API change.

Status of this Pull Request

Untested as I don't use RPC.

What is missing until this pull request can be merged?

Seems trivial, so I'd just do it. I've been running builds with this change in (but without using the RPC API).

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 pljones force-pushed the bugfix/rpc-remaining-directory-server-mentions branch from 26e4cf0 to 352b687 Compare April 16, 2023 15:25
@pljones pljones added refactoring Non-behavioural changes, Code cleanup needs documentation PRs requiring documentation changes or additions JSON-RPC Related to the JSON-RPC API labels Apr 17, 2023
@pljones pljones added this to the Release 3.10.0 milestone Apr 17, 2023
@pljones pljones requested a review from hoffie April 17, 2023 19:04
@pljones
Copy link
Collaborator Author

pljones commented Apr 17, 2023

@dtinth I'd also appreciate your review, but I can't add you as a reviewer for some reason.

@pljones pljones self-assigned this Apr 17, 2023
dtinth
dtinth approved these changes Apr 17, 2023
View reviewed changes
Copy link
Contributor

@dtinth dtinth left a comment

Choose a reason for hiding this comment

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

LGTM.

Btw, this is a breaking change for consumers, but since JSON-RPC is “experimental” it does not need to obey semver. Still, it may be worth mentioning in the changelog to make this change less surprising.

@pljones
Copy link
Collaborator Author

pljones commented Apr 17, 2023

Btw, this is a breaking change for consumers, but since JSON-RPC is “experimental” it does not need to obey semver. Still, it may be worth mentioning in the changelog to make this change less surprising.

Yeah, true. Done.

@pljones pljones merged commit 20148a7 into jamulussoftware:main Apr 17, 2023
@pljones pljones deleted the bugfix/rpc-remaining-directory-server-mentions branch April 17, 2023 19:48
@ann0see
Copy link
Member

ann0see commented Jul 24, 2023

I think everything is documented here too? @pljones please drop needs documentation if you think it's valid to drop.

@pljones
Copy link
Collaborator Author

pljones commented Jul 25, 2023

Mmm, the doc-gen stuff is there. We don't do jamuluswebsite doc for JSONRPC, right?

@ann0see
Copy link
Member

ann0see commented Jul 25, 2023

We don't do jamuluswebsite doc for JSONRPC, right?

True. There's not much on the website. The actual documentation of the API is currently in the repo.

@pljones pljones removed the needs documentation PRs requiring documentation changes or additions label Jul 25, 2023
@pljones
Copy link
Collaborator Author

pljones commented Jul 25, 2023

Unlabelled, then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hoffie hoffie Awaiting requested review from hoffie

1 more reviewer

@dtinth dtinth dtinth approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@pljones pljones

Labels

JSON-RPC Related to the JSON-RPC API 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.

3 participants

@pljones @ann0see @dtinth