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.

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 ci workflow to publish docs #489

Closed
wants to merge 9 commits into from

Conversation

varshith257
Copy link

@varshith257 varshith257 commented Nov 8, 2024

/claim #458
Closes #458

@varshith257
Copy link
Author

@neko-kai Could you have a look at secrets needed?

@pshirshov
Copy link
Member

Well, all the secrets are managed at the org level :)

Also, may I ask you to not change the formatting please? This style differs from the default Idea formatter, so it would be annoying and it doesn't really bring any value.

@varshith257
Copy link
Author

varshith257 commented Nov 8, 2024

@pshirshov I think they got changed with my Default yml formatter on my IDE 😄.

I will revert those format issues

@pshirshov
Copy link
Member

Once you rebase, the build should be green.

Though there is a bit more in this. This comment remains valid: #339 (comment)

We need to do more than just publishing our readme, we should create a documentation stub ( which will be used for #340 ). There is something created by the previous contributor in the ./docs folder but it's unmaintained.

We need to find a way to both have a nice readme and include a part of it into our documentation. I believe, there are no inclusions in Github markdown, so we need something like that:

  1. Our current readme remains as it is
  2. We have a documentation stub in ./docs folder.
  3. The doc publishing job takes our readme, strips irrelevant parts (we need to figure out how to mark them) and inserts into the docs which are about to be published. Currently I would like to exclude the "build" and the github badges.

@pshirshov
Copy link
Member

pshirshov commented Nov 8, 2024

Regarding marking the relevant and irrelevant parts, github markdown supports comments, so probably we should just have lines like <!--- docs:start ---> and <!--- docs:end--->, plus some sed/awk magic.

Copy link
Member

@pshirshov pshirshov left a comment

Choose a reason for hiding this comment

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

See my comments

@varshith257
Copy link
Author

@pshirshov Does it mean to eradicate ./docs folder?

@pshirshov
Copy link
Member

Another collaborator already opened a P/R which looks correct: #491

I don't know how to handle this situation, so probably I'll merge the one which is completed first.

@varshith257
Copy link
Author

varshith257 commented Nov 10, 2024

@pshirshov If the PR isn't got reviewed and if another contributor opened a PR alongside me for the same issue that's ok and reviewer goes with which matches close to the solution.

#491 is entirely built upon my PR reviews and this PR isn't stale 🤦

I don't what @Dhanus3133 thought to open an another PR with it 🤔

@Dhanus3133
Copy link
Contributor

@varshith257 I noticed that the issue was still open, so I wanted to raise a PR that could close the issue itself. Yes, I saw the comments on your PR by the maintainer and implemented them the way they need accordingly. I want to clarify that I didn't copy any of your code; I developed my solution independently using references from other Zio's repositories. My intention wasn't to undermine your work but to solve the issue.

Since I didn't copy or use any of your code as a reference, I'm happy to let the maintainer decide how to proceed, and I'm fine with whatever they choose.

@varshith257
Copy link
Author

varshith257 commented Nov 10, 2024

@Dhanus3133 I am not pretended you copied my code . My PR is already in the way of solving, and got reviews and you again made a duplicate efforts with solving review comments

Anyway the better thing here is let's collaborate on this issue .

@Dhanus3133
Copy link
Contributor

Anyway the better thing here is let's collaborate on this issue .

Since the issue is almost resolved, collaboration isn’t needed here, but I’m open to teaming up on future issues!

@varshith257
Copy link
Author

varshith257 commented Nov 10, 2024

@pshirshov Since the other contributor worked upon this PR review comments(as the PR hasn't gone inactive though), I wished to collaborate rather than make duplicate efforts on both ends(AFAICT the other contributor isn't open to collaborating)

I don't know how to handle this situation

I have been working and contributing to ZIO for the past 5 months and I have also seen a similar kind of situation, you can see collaborations here(where I have worked on reviewer comments and collaborated with other contributor who also worked on it)
zio/zio#8973

[PS: I am not going for tik for tack, just to say everyone's efforts are valued, so I welcomed other contributor for collaborating ]

@Dhanus3133
Copy link
Contributor

@varshith257 the recent commit seems to be an exact copy of my PR #491, including all the CI updates, as seen in your recent commit (5f88e20). Could you please clarify?

Cause this particular change you made was not even in the reviewer's comment (also you would say it's reference from other repo but still)

if: ${{ ((github.event_name == 'release') && (github.event.action == 'published')) || (github.event_name == 'workflow_dispatch') }}

Ultimately, I didn't copy any of your code in the PR, but you choose to do this after asking this question. It’s up to the maintainer to decide how to proceed.

@varshith257
Copy link
Author

varshith257 commented Nov 10, 2024

Yeah, this is what I concerned now about what you commented ( I am not going for tick for tack , just to know you the efforts of everyone are valued)

Yes, I saw the comments on your PR by the maintainer and implemented them the way they need accordingly.

I do argue the same of I have worked upon my review comments and I saw the comments on your PR by the maintainer and implemented them the way they need accordingly to close the issue

@pshirshov
Copy link
Member

I've merged #491, I hope I didn't offend anyone... @jdegoes : I'm not sure how to handle situations like these...

@varshith257
Copy link
Author

varshith257 commented Nov 10, 2024

@pshirshov Nope! I am fine with it and respect your opinion. Happy to contribute to other issues :)

@varshith257 varshith257 deleted the publish-docs branch November 10, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish Project Docs On Release Events
3 participants