Skip to content
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

Docs for Unreal Engine SDK #312

Merged
merged 13 commits into from
Jan 22, 2024
Merged

Docs for Unreal Engine SDK #312

merged 13 commits into from
Jan 22, 2024

Conversation

pasotee
Copy link
Contributor

@pasotee pasotee commented Sep 18, 2023

Description

First iteration for the Unreal Engine SDK docs.

Requirement checklist

  • I have validated my changes on a test/local environment.
  • I have checked the SNYK/Dependabot reports and applied the suggested changes.
  • (Optional) I have updated outdated packages.

@pasotee pasotee requested review from sigewuzhere and a team as code owners September 18, 2023 06:15
@pasotee pasotee force-pushed the unreal-engine-docs branch 2 times, most recently from b191eb2 to f9ff47c Compare January 15, 2024 16:39
@pasotee pasotee changed the title base copy for Unreal + adding references everywhere Docs for Unreal Engine SDK Jan 15, 2024
@pasotee
base copy for Unreal + adding references everywhere
5d2fe6a 
first example

done up to line 169

Line 253

line 273

line 315

logging done 315/658

Polling modes done - 369/655


Delegates done

offline mode done 433/668


http time and proxy 433/645

force refresh + skipping custom cache for now.

GetAllValueDetails done

get all values done

get all keys done - 435/572

all lines done
@pasotee
TODO cleanup before publishing
b51fa2a
@pasotee
added marketplace link
1bd23e9
@pasotee
deleted empty sections
fbd3ce4
@pasotee pasotee force-pushed the unreal-engine-docs branch from 899edbb to 48feff3 Compare January 18, 2024 13:01
@pasotee pasotee force-pushed the unreal-engine-docs branch from 48feff3 to 060b447 Compare January 18, 2024 13:02
@pasotee @kp-cat
accepted PR suggestions
e7348b0 
Co-authored-by: Peter Adam Korodi <[email protected]>
kp-cat
kp-cat reviewed Jan 18, 2024
View reviewed changes
website/docs/sdk-reference/unreal.md Outdated
| `ConnectTimeoutMs` | Optional, defaults to `8000ms`. Sets the amount of milliseconds to wait for the server to make the initial connection (i.e. completing the TCP connection handshake). `0` means it never times out during transfer |
| `ReadTimeoutMs` | Optional, defaults to `5000ms`. Sets the amount of milliseconds to wait for the server to respond before giving up. `0` means it never times out during transfer. |
| `PollingMode` | Optional, sets the polling mode for the client. [More about polling modes](#polling-modes). |
| `AutoPollInterval` | For PollingMode == Custom, sets at least how often this policy should fetch the latest configuration and refresh the cache. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `AutoPollInterval` | For PollingMode == Custom, sets at least how often this policy should fetch the latest configuration and refresh the cache. |
| `AutoPollInterval` | For PollingMode == Custom, sets at least how often this policy should fetch the latest config JSON and refresh the cache. |

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed this suggestion, I will apply the change manually next commit.

laliconfigcat
laliconfigcat requested changes Jan 19, 2024
View reviewed changes
Copy link
Member

@laliconfigcat laliconfigcat left a comment

Choose a reason for hiding this comment

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

Looks good to me, but we had some changes regarding internal links. Could you please change them?

website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
@pasotee
Copy link
Contributor Author

pasotee commented Jan 19, 2024

Sure thing. I've committed the suggested edits and fixed one more link I found locally using the old format.

kp-cat
kp-cat previously approved these changes Jan 19, 2024
View reviewed changes
laliconfigcat
laliconfigcat previously approved these changes Jan 19, 2024
View reviewed changes
sigewuzhere
sigewuzhere suggested changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@sigewuzhere sigewuzhere left a comment

Choose a reason for hiding this comment

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

Hey @pasotee !
A few links are missing from:

  • sidebars.js
  • index.js

Something is wrong with the header:
image

website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
website/docs/sdk-reference/unreal.md Outdated

## Custom Cache

**Coming soon**: [https://github.com/configcat/unreal-engine-sdk/issues/9](Support for Custom Cache #9)
Copy link
Contributor

Choose a reason for hiding this comment

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

image

Improperly formatted link

website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
website/docs/sdk-reference/unreal.md Outdated Show resolved Hide resolved
@pasotee pasotee dismissed stale reviews from laliconfigcat and kp-cat via 7203ac6 January 22, 2024 10:21
@pasotee
Copy link
Contributor Author

pasotee commented Jan 22, 2024

hey @sigewuzhere, thanks for taking a look. I've applied your suggestions and fixed the broken links.

Regarding the links (sidebar.js & index.js), I've removed them on purpose. I will submit the SDK for approval from the Unreal team (they will use a static link to access to documentation during the review process) and once it's publicly available, I will make a follow-up PR to add the missing links.

Regarding the broken header, it looks like there is an extra n at the start of the markdown file like this:

---
id: unreal
title: Unreal Engine SDK Reference
description: ConfigCat Unreal Engine SDK Reference. This is ...
---

to

n---
id: unreal
title: Unreal Engine SDK Reference
description: ConfigCat Unreal Engine SDK Reference. This is ...
---

I don't see it on the GitHub version, can you confirm if this is a local problem? Otherwise I will try to dig deeper into it.

@kp-cat kp-cat requested a review from sigewuzhere January 22, 2024 11:01
sigewuzhere
sigewuzhere approved these changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@sigewuzhere sigewuzhere left a comment

Choose a reason for hiding this comment

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

Hi @pasotee ! Thanks, yeah the n was a local mistake, sorry about that. All good to go from my end.

@kp-cat kp-cat merged commit a746c5a into configcat:master Jan 22, 2024
2 checks passed
@pasotee pasotee deleted the unreal-engine-docs branch January 22, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kp-cat kp-cat kp-cat left review comments

@sigewuzhere sigewuzhere sigewuzhere approved these changes

@laliconfigcat laliconfigcat laliconfigcat left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@pasotee @kp-cat @sigewuzhere @laliconfigcat