Skip to content

add sdk design doc #235

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jul 24, 2023
Merged

add sdk design doc #235

merged 8 commits into from
Jul 24, 2023

Conversation

MichaelClifford
Copy link
Collaborator

This PR adds a design doc for the SDK and closes #158

@MichaelClifford MichaelClifford force-pushed the sdkArchDoc branch 2 times, most recently from 2fef297 to 561486d Compare July 19, 2023 20:24
Maxusmusti
Maxusmusti requested changes Jul 19, 2023
View reviewed changes
Copy link
Collaborator

@Maxusmusti Maxusmusti left a comment

Choose a reason for hiding this comment

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

A few more tiny nits I missed when going through the original doc, but otherwise LGTM!

@MichaelClifford
Copy link
Collaborator Author

Thanks for catching those @Maxusmusti . All should be fixed now. :)

Maxusmusti
Maxusmusti reviewed Jul 20, 2023
View reviewed changes
Copy link
Collaborator

@Maxusmusti Maxusmusti left a comment

Choose a reason for hiding this comment

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

LGTM, just looks like it needs a rebase

@Maxusmusti
Copy link
Collaborator

Oh, one thing to be careful of is storing things that aren't part of the code documentation in docs, as with each release that dir gets overwritten with updated docs

@MichaelClifford
Copy link
Collaborator Author

where should we store images for documentation then? I don't want to cause confusion overloading the term "image" in the top directory

@Maxusmusti
Copy link
Collaborator

hmm, maybe an assets dir would be good to have? Or something along those lines

@MichaelClifford
Copy link
Collaborator Author

👍 assets is good

@MichaelClifford
Copy link
Collaborator Author

@Maxusmusti Should be good. Let me know if there is anything else

tedhtchang
tedhtchang requested changes Jul 21, 2023
View reviewed changes
@MichaelClifford MichaelClifford force-pushed the sdkArchDoc branch 2 times, most recently from b42b0b7 to b96caa4 Compare July 21, 2023 15:16
anishasthana
anishasthana suggested changes Jul 24, 2023
View reviewed changes
@MichaelClifford
Copy link
Collaborator Author

Thanks for the review @anishasthana, changes made

anishasthana
anishasthana approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@anishasthana anishasthana left a comment

Choose a reason for hiding this comment

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

/lgtm

Maxusmusti
Maxusmusti approved these changes Jul 24, 2023
View reviewed changes
Copy link
Collaborator

@Maxusmusti Maxusmusti left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelClifford MichaelClifford merged commit 4d523cc into project-codeflare:main Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anishasthana anishasthana anishasthana approved these changes

@tedhtchang tedhtchang tedhtchang approved these changes

@Maxusmusti Maxusmusti Maxusmusti approved these changes

@accorvin accorvin Awaiting requested review from accorvin

@KPostOffice KPostOffice Awaiting requested review from KPostOffice

@dimakis dimakis Awaiting requested review from dimakis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SDK Architecture Document
4 participants
@MichaelClifford @Maxusmusti @anishasthana @tedhtchang