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

Remove docs directory from codebase #1750

Closed
literalEval opened this issue Apr 5, 2023 · 17 comments · Fixed by #1760
Closed

Remove docs directory from codebase #1750

literalEval opened this issue Apr 5, 2023 · 17 comments · Fixed by #1760
Assignees
Labels
documentation Update documentation

Comments

@literalEval
Copy link
Member

Currently, we have a docs folder in the root of the codebase, which contains the documentation generated by dart doc. This issue proposes to remove this directory entirely.

The reasons can be explained as:

  1. The directory is currently of about ~20mbs. That's a waste of bandwidth and space.
  2. Everything that the doc mentions is already in the codebase and can be generated by dart doc on the go. Anyone can do this on their local machine.
  3. We are already hosting our documentation on Talawa-docs
@github-actions github-actions bot added documentation Update documentation unapproved Unapproved, needs to be triaged labels Apr 5, 2023
@palisadoes
Copy link
Contributor

How are you going to modify this file to make sure Talawa-Docs gets updated without the docs/ directory?

@literalEval
Copy link
Member Author

The workflow will run as usual and the docs will be copied as expected. We just won't save the output doc anywhere in the Talawa Codebase. Just like the Build workflow builds and generates apk for Android, but we never actually save it anywhere (though we have opened a new issue for it, to explicitly save the build).

@palisadoes
Copy link
Contributor

@anwersayeed Please review this PR when it's submitted.

@palisadoes palisadoes removed the unapproved Unapproved, needs to be triaged label Apr 5, 2023
@anwersayeed
Copy link

@literalEval Please create a PR for the substitute option you are recommending before deleting the docs folder.

@literalEval
Copy link
Member Author

@anwersayeed sure

@literalEval
Copy link
Member Author

literalEval commented Apr 9, 2023

@anwersayeed can you please confirm whether the update docs workflow works for now or not? I can't see it updating the documentation in Talawa Docs. The workflow does run but changes nothing.

Edit: A closer look at the workflow reveals that the files are no copied at all. Like in this PR workflow run, docs were changed a bit, but the workflow run says that "No changes detected".

Screenshot_20230409_130730

@anwersayeed please correct me if I am wrong.

@anwersayeed
Copy link

@literalEval It commits only when it detects any change in the documentation!

@literalEval
Copy link
Member Author

@anwersayeed that I understand :)
But how does it define "change in documentation?" Because the PR I have mentioned above did change the documentation of some fields. Not only that specific PR, but all of the PRs I saw had the same message of "no documentation change" even though documentation was changed for fields. That is why Talawa Docs has quite old documentation of the codebase.

@anwersayeed
Copy link

@literalEval We have a folder talawa/docs. A documentation change will be detected when files inside this folder are changed.

@literalEval
Copy link
Member Author

@anwersayeed Oh okay. So we generate the docs, match them with the ones in the talawa/docs folder, and if changes are detected, we push to Talawa Docs. Is that correct ?

@anwersayeed
Copy link

@anwersayeed that I understand :) But how does it define "change in documentation?" Because the PR I have mentioned above did change the documentation of some fields. Not only that specific PR, but all of the PRs I saw had the same message of "no documentation change" even though documentation was changed for fields. That is why Talawa Docs has quite old documentation of the codebase.

Checkout this workflow, Update-Documentation. It seems docs/talawa are not receiving changes.

@anwersayeed
Copy link

@anwersayeed Oh okay. So we generate the docs, match them with the ones in the talawa/docs folder, and if changes are detected, we push to Talawa Docs. Is that correct ?

Yeah, we cannot commit without any changes.

@literalEval
Copy link
Member Author

literalEval commented Apr 9, 2023

How about we match the doc generated during workflow build, and the one in the Talawa Docs repo? Because we are generating the docs and cloning Talawa Docs anyway, so they will be no new overhead. That way we can get rid of having the docs directory in the codebase completely, and ensure that we always have the latest documentation build without depending on the contributor to do so. What do you think @anwersayeed ?

@anwersayeed
Copy link

How about we match the doc generated during workflow build, and the one in the Talawa Docs folder? Because we are generating the docs and cloning Talawa Docs anyway, so they will be no new overhead. That way we can get rid of having the docs directory in the codebase completely, and ensure that we always have the latest documentation build without depending on the contributor to do so. What do you think @anwersayeed ?

Perfect, that is what I initially thought. Please go ahead.

@literalEval
Copy link
Member Author

@anwersayeed but the current implementation too seems to be trying to do this.

Screenshot_20230409_170933

According to the above snippet, the documentation is generated and then we check whether it changed any files or not. That should do be doing its job at least, because the logic to check changes is also correct. I doesn't depend on the contributor to do so. Why does it show "no doc change" then ?

@literalEval
Copy link
Member Author

@anwersayeed @palisadoes The steps output in the GitHub workflow is deprecated, and was not working in our workflow for some months. Migrating to the newer alternative causes everything to work fine now :)

@literalEval
Copy link
Member Author

@palisadoes I fixed this locally. We don't need the docs directory anymore :)
Opening the PR. It is related to the PUSH workflow, so you might need to merge many times :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Update documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants