Skip to content

docs(server): new docs site for ARFlow server#2

Merged
YiqinZhao merged 7 commits intocake-lab:mainfrom
FelixNgFender:main
Sep 4, 2024
Merged

docs(server): new docs site for ARFlow server#2
YiqinZhao merged 7 commits intocake-lab:mainfrom
FelixNgFender:main

Conversation

@FelixNgFender
Copy link
Contributor

@FelixNgFender FelixNgFender commented Aug 30, 2024

I replaced the static.yml CI action and the bash scripts used to build & copy the docs to the static website. We now run all of that on a GitHub runner with a new website.yml action.

Added docs for Protobuf schema, 1 action item left here though: the old message field in RegisterResponse has been changed in the Protobuf schema, but the change has not been reflected in the generated data access layer for the C# client.

message RegisterResponse {
    string uid = 1; // previously named `message`
}

Centralized all remaining bash scripts in python/ to Poetry scripts inside pyproject.toml.

Added docs for important packages & tools used in the server.

@YiqinZhao YiqinZhao self-requested a review August 30, 2024 13:03
@YiqinZhao
Copy link
Member

YiqinZhao commented Aug 30, 2024

Thanks for the contribution, a few thoughts of mine:

  • Why is the message field changed to uid? To align with DataFrameRequest?
  • I don't think centralizing all bash script code to the pyproject.toml file is a good practice. The Python side code will be published to pypi as both a package and a CLI tool. The command defined in the pyproject.toml file will be exposed as CLI subcommand, for example: arflow serve. The scripts for executing example code shouldn't be added to there.

YiqinZhao
YiqinZhao reviewed Aug 30, 2024
View reviewed changes
YiqinZhao
YiqinZhao reviewed Aug 30, 2024
View reviewed changes
Copy link
Member

@YiqinZhao YiqinZhao left a comment

Choose a reason for hiding this comment

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

Added more comments

YiqinZhao
YiqinZhao reviewed Aug 30, 2024
View reviewed changes
@YiqinZhao
Copy link
Member

YiqinZhao commented Aug 30, 2024

The changes look good now. Will merging this PR break the server? Because the uid field updates are not reflected on the Unity side.

@YiqinZhao YiqinZhao added WPI Projects created by WPI students. MQP Project related to WPI MQP projects. labels Sep 1, 2024
YiqinZhao
YiqinZhao approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member

@YiqinZhao YiqinZhao left a comment

Choose a reason for hiding this comment

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

LGTM

@YiqinZhao YiqinZhao merged commit 7c13069 into cake-lab:main Sep 4, 2024
legoeruro added a commit to legoeruro/ARFlow that referenced this pull request Oct 11, 2024
@legoeruro
Merge pull request cake-lab#2 from FelixNgFender/non-streaming-data-m…
6204180 
…odalities

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

Reviewers

@YiqinZhao YiqinZhao YiqinZhao approved these changes

Assignees

No one assigned

Labels

MQP Project related to WPI MQP projects. WPI Projects created by WPI students.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@FelixNgFender @YiqinZhao