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

chore/remove unused pages #250

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

Magnus-Kuhn
Copy link
Contributor

No description provided.

@Magnus-Kuhn Magnus-Kuhn marked this pull request as ready for review August 21, 2024 13:07
@jkoenig134
Copy link
Member

jkoenig134 commented Aug 22, 2024

the pr is called remove unused pages - I only see diagrams removed. Is this not finished or named confusingly?

Edit: ignore this, I only saw the changes from last commit.

jkoenig134
jkoenig134 previously approved these changes Aug 22, 2024
Copy link
Member

@jkoenig134 jkoenig134 left a comment

Choose a reason for hiding this comment

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

LGTM. @britsta do you want to have another look?

@britsta
Copy link
Contributor

britsta commented Aug 22, 2024

LGTM. @britsta do you want to have another look?

I'll take a quick look at it now and then approve it or comment on it.

@jkoenig134 jkoenig134 requested a review from britsta August 22, 2024 09:31
@jkoenig134
Copy link
Member

thanks!

Copy link
Contributor

@britsta britsta left a comment

Choose a reason for hiding this comment

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

Apart from the prerelease files that we should not delete IMO (see other comments), there are some files here that I agree in principle to delete within the documentation repository, as this is not intended as a storage for drafts, but which nevertheless contain some valuable content. I will store this content, which we could include elsewhere in the documentation, on suitable Confluence pages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove this file. It will automatically created anyway the next time the Excel script is running. The content here was written 5 months ago and it is quite possible that this prerelease documentation is currently not published due to a missing entry ("true" in the "published" column) in the Excel table. I don't think it gives us any advantage to delete this content here if the file is going to be recreated anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove this file. It will automatically created anyway the next time the Excel script is running.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove this file. It will automatically created anyway the next time the Excel script is running. The content here was written 5 months ago and it is quite possible that this prerelease documentation is currently not published due to a missing entry ("true" in the "published" column) in the Excel table. I don't think it gives us any advantage to delete this content here if the file is going to be recreated anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove this file. It will automatically created anyway the next time the Excel script is running. The content here was written 5 months ago and it is quite possible that this prerelease documentation is currently not published due to a missing entry ("true" in the "published" column) in the Excel table. I don't think it gives us any advantage to delete this content here if the file is going to be recreated anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove this file. It will automatically created anyway the next time the Excel script is running. The content here was written 5 months ago and it is quite possible that this prerelease documentation is currently not published due to a missing entry ("true" in the "published" column) in the Excel table. I don't think it gives us any advantage to delete this content here if the file is going to be recreated anyway.

@Magnus-Kuhn
Copy link
Contributor Author

Apart from the prerelease files that we should not delete IMO (see other comments)

I selected them for deletion because the documentation has grown in a way that there would be no place for them anymore without a major rewrite (mostly swallowed by the Working with Requests section), and there was no special content that would be an effort to recreate.

And the excel can always be adapted, in fact I'd like to add a column there that the pages where nothing is on them aren't there (or they're put into gitignore)

@jkoenig134 jkoenig134 dismissed their stale review August 22, 2024 10:30

too fast given

@britsta
Copy link
Contributor

britsta commented Aug 22, 2024

I selected them for deletion because the documentation has grown in a way that there would be no place for them anymore without a major rewrite (mostly swallowed by the Working with Requests section), and there was no special content that would be an effort to recreate.

You are probably right that these contents do not quite fit into the current state of the documentation. However, I think that is the reason why they were declared as prereleases and then they can be revised at a later date. I remember that there was a reason why we wanted to have at least a little bit of content on some scenario pages so that the Integrators could at least find a little bit of information there as we expose links to these preleases on certain websites.

And the excel can always be adapted, in fact I'd like to add a column there that the pages where nothing is on them aren't there (or they're put into gitignore)

That would be an option. We should discuss this again separately.

@jkoenig134
Copy link
Member

I will transform this to a draft, b/c it seems like it wasn't properly discussed if this should happen and if yes in which extent.

@jkoenig134 jkoenig134 marked this pull request as draft August 28, 2024 10:04
@jkoenig134 jkoenig134 requested review from jkoenig134 and britsta and removed request for stnmtz, tnotheis, jkoenig134 and britsta August 28, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants