-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Rewrite documentation with more specific use cases in mind #119
base: main
Are you sure you want to change the base?
Conversation
ae65670
to
0f825e4
Compare
0f825e4
to
503de02
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #119 +/- ##
=======================================
Coverage 93.38% 93.38%
=======================================
Files 9 9
Lines 1800 1800
=======================================
Hits 1681 1681
Misses 119 119 Continue to review full report in Codecov by Sentry.
|
README.md
Outdated
Note that famedly-sync is currently not designed to be deployed to | ||
projects with existing users, or projects that have been synced to | ||
with a different source before. | ||
|
||
Since famedly-sync relies on metadata to perform its tasks, if users | ||
already exist in the instance, they may be deleted by a sync, or cause | ||
desync issues if they happen to be users with elevated permissions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this a [!CAUTION] and emphasize the potential destructive outcome.
```yaml | ||
services: | ||
famedly-sync-agent: | ||
image: docker-oss.nexus.famedly.de/famedly-sync-agent:<version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably split this out into a separate issue, but we should really standardise the name into just famedly-sync
without agent
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would also be good to expand the first part of the README: What does famedly-sync do (and not do)?
What are the potential footguns? How do you troubleshoot problems?
What are the known limitations or design considerations? Are ldap group structures maintained? can you use all three data sources simultaneously? will famedly-sync automagically figure out the special format of our AD that's been running since 1993? etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this PR focuses on the deployment flow, since infra specifically asked for it - I think we should split this out into a separate ticket so we get the quick win from clearer deployment docs?
FWIW, this should probably make it into more general user docs, but adding it to the readme while we don't have a clear process for that seems reasonable.
f04a6b8
to
4e83c18
Compare
4e83c18
to
b5257df
Compare
We've been told by users that a more step-by-step guide for deployment would be helpful. The current docs cover most of this, but it's a bit disjoint. Having clear bullet points of what to prepare and do would probably help.