-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add collaboration workflow document #2507
Conversation
6d54a25
to
80c7f32
Compare
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.
Here are some preliminary thoughts for you to consider. I only realized that the docs were still a WIP after seeing the image hadn't made it in yet.
docs/collaboration.md
Outdated
|
||
That means including them in the repository for the project or including them | ||
in the `files` attribute in the configuration so they are available in the | ||
content bundle. |
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.
With my earlier comment in mind, maybe start with a publisher creating a project that is not being collaborated on and then take the next steps to make sure it can be collaborated on, then switch to the collaborator's tasks?
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.
That is interesting. I don't want to repeat too much that is in the "Publishing from VS Code or Positron" document but I do like the idea of starting from the point of view of setting up a project for collaboration.
docs/collaboration.md
Outdated
content bundle from the latest Source Version on Posit Conect. | ||
|
||
Doing so ensures that you have the most up-to-date content, but also | ||
configuration settings and deployment details. |
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.
We should also consider some of the impacts that the different content types have on collaboration, such as the interpreter versions and if dependencies are being updated.
This should include a special clarification for R environments, where for now, collaborators need to agree upon runtime versions and maintain the appropriate package installations.
It's less of an issue for Python and Quarto, so I think we can skim over the details for them, while still wanting to promote best practices of using the same versions while collaborating.
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.
Definitely. I think a note about how package files, and versions in the configuration would be good to include.
428fbd0
to
924979f
Compare
924979f
to
050e485
Compare
I ended up removing the section about first-time deployment without a credential, but if we want this included I can update the screenshot. |
docs/collaboration.md
Outdated
Posit Publisher creates and utilizes two files for deploying content. Both are | ||
inside of the `.posit/publish` directory in the same directory as | ||
your entrypoint file. |
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 we mention here about adding these to version control?
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 mentioned it at the beginning of the source control section, but we could definitely move it up. I tried to keep that separate to avoid bouncing any user that wasn't using source control.
docs/collaboration.md
Outdated
If you and your collaborators aren't using source control, a content bundle | ||
containing the source code and data for a piece of content can be downloaded | ||
from Posit Connect. Content bundles are created when content is published and | ||
they work great with Posit Publisher. |
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.
a content bundle containing the source code and data for a piece of content can be downloaded from Posit Connect
I'm not sure we need to be absolutely pedantically correct in this document, but this is true if and only if someone uploaded the source code alongside the bundle. There are a number of circumstances where people don't (e.g. because the computation to render takes too long, the render requires access to systems that the connect server doesn't have).
Content bundles are created when content is published and they work great with Posit Publisher.
I don't fully understand what's being conveyed here. That the bundle is created as a (hidden) step when publishing and so even if one has never gone and made a bundle on their own, they still have them? Something else?
As it reads now, it sounds to me almost like "once the content gets to the Connect server it makes a bundle of the source code of whatever produced it" which isn't quite accurate. But also maybe this is slightly duplicative of the sentence above so could be just deleted?
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.
if the content author did not include the source code, is it possible to collaborate in this way?
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'm not sure we need to be absolutely pedantically correct in this document, but this is true if and only if someone uploaded the source code alongside the bundle. There are a number of circumstances where people don't
I pulled this language straight from the document I link to below to keep things uniform in explanation if that link is followed to understand more. We could be more Publisher specific instead and say that the content bundle includes all of the included files sent to Connect, but that isn't absolutely pedantically correct either.
As it reads now, it sounds to me almost like "once the content gets to the Connect server it makes a bundle of the source code of whatever produced it" which isn't quite accurate. But also maybe this is slightly duplicative of the sentence above so could be just deleted?
I didn't think that first sentence enforced that every piece of published content had a content bundle to be potentially utilized with Publisher. It felt vague enough that if you didn't know what a content bundle was it wasn't clear that one was created on deploy. It is a bit duplicative so we could remove it if we think the first sentence is sufficient.
if the content author did not include the source code, is it possible to collaborate in this way?
No it is not possible. That is why the [!NOTE]
below was written to call out that this method only includes files actually sent to Connect.
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.
No it is not possible. That is why the [!NOTE] below was written to call out that this method only includes files actually sent to Connect.
OOOOOOH that's what it was supposed to be noting. I didn't make that connection at all when I read this through the first time, so I think we do need to do some work to tie those things together
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've written the "Without source control" section based on all of the feedback removing both the note, and trying to get across the information in a more condensed and simpler way.
Let me know if you think it communicates the intended information better.
docs/collaboration.md
Outdated
> [!NOTE] | ||
> The content bundle will only contain files sent to Posit Connect. Be sure any | ||
> files that you would like collaborators to have access to are in the | ||
> Configuration's included `files`. | ||
> For more details see the [Project Files documentation for Posit Publisher](./vscode.md#project-files). |
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 know we don't (yet) have a huge number of call outs in the publisher doc, but across the Connect docs we have a lot and so are trying to eliminate them. Could we make this not-a-note?
Also: I'm not totally sure this is necessary here in the docs? It's absolutely true that if some file is important for collaboration they need to include it, but also if they are missing it from the deploy it's almost certainly going to fail to deploy at all, right? And (like linked there) we talk about project files when deploying elsewhere. This isn't a unique circumstance in the collaboration flow. So maybe we 🚮 and don't need to think about is it a note or not?
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 this is a strong enough reminder to warrant a note. If we change the language, as suggested in #2507 (comment), to explain that the content bundle contains all files sent to Connect then I think we could pull the call out styling, but I think the "Be sure any files that you would like collaborators to have access to are in the Configuration's included files
." language along with the link is particularly helpful.
also if they are missing it from the deploy it's almost certainly going to fail to deploy at all, right?
I think this is mostly true for content that runs on Connect, but if a static project is generated and those built files published (like a Quarto site) then the files needed to collaborate aren't necessary for the deployment to succeed. If not collaborating it is very reasonable to not include them.
I also know a lot of unnecessary files can be included in source control like READMEs, it could be true that some would want those to collaborate if not using source control.
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 removed the note in #2507 (comment) trying to collapse it into the information above.
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.
Sorry for all the notifications — I wanted to make sure these got to you even if I didn't finish the full review. I did get through the whole thing now though. Thank you for all the work you put in here, this is a big chunk of text and I like the overall flow.
de88b38
to
53ad653
Compare
53ad653
to
bbd41ba
Compare
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.
This is looking great, I have a few more comments, but most are very very minor, stylistic, or the take it or leave it kind
This PR adds a document describing the collaboration flow we envision for Publisher as well as a section in the extension read me as a feature.
It isn't clear if this document will actually live in the repo, or in the Posit Connect docs. This is put up here for review.
Tackles #2383