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

docs: Fix incorrect on-premise refs from PR #1023 #1097

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexmalins
Copy link

#1023 had a small number of mistakes, the most import of which broke the image links on the Examples page for the AWS on-premises examples

This PR corrects the issues and uses on-premises (or one instance of on-premise in the changelog) where needed

Copy link
Collaborator

@gabriel-tessier gabriel-tessier left a comment

Choose a reason for hiding this comment

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

  • Revert changes in the CHANGELOG.md file as the changes are related to what changed in the version 0.16.

@gabriel-tessier
Copy link
Collaborator

@alexmalins
Thank you for the PR, good catch.

Can you check the changes asked in the review when you will have time.

@gabriel-tessier gabriel-tessier added the kind/docs Improvements or additions to documentation label Feb 11, 2025
@alexmalins
Copy link
Author

alexmalins commented Feb 12, 2025

hey @gabriel-tessier ! thanks for the review

Revert changes in the CHANGELOG.md file as the changes are related to what changed in the version 0.16.

Are you sure this is the right thing to do?

0.16.0 made the breaking change from diagrams.oci.connectivity import Customerpremisesequip to from diagrams.oci.connectivity import CustomerPremise which is documented here.

0.24.0 (incorrectly documented as 0.23.5 in the changelog) made the second breaking change from #1023 of from diagrams.oci.connectivity import CustomerPremise to from diagrams.oci.connectivity import CustomerPremises. However 360c6bc from #1023 incorrectly documented that change as from diagrams.oci.connectivity import CustomerPremises to from diagrams.oci.connectivity import CustomerPremises (i.e. Premises plural both both and after), which is what my pre-review state a54a3cc fixes.

A wider issue is the changelog hasn't been well maintained since the v0.17.0 release. None of the subsequent releases are documented in the change log, other than @filipeaaoliveira 's work in #1023 - which was put in at the bottom of the changelog instead of at the more usual top position.

If you like I could add another commit to bring the changelog up to date, by back filling the changes since v0.17.0 and reordering everything correctly?

@gabriel-tessier
Copy link
Collaborator

@alexmalins

Yep sorry, as you noticed the file is no more used and when I asked filipeaaoliveira to add the changelog in the PR review I forget that mingrammer were no more using the file.

Sorry for the confusion, so to sum things up:

  • CHANGELOG.md file is no more used
  • I confirm you can revert the changes without problem.

After that I will just wait for another maintainer to review and your changes will be merged in master.
Thank you again for the PR and sorry again for the confusion, it was better for me to inform about the changelog in the first review.

import of which broken the image links on the
Examples page for the AWS on-premises examples

This PR corrects the issues and uses on-premises
where needed
@alexmalins alexmalins force-pushed the fix-broken-on-premises-img-links branch from a54a3cc to ff174e3 Compare February 12, 2025 12:36
@alexmalins
Copy link
Author

Understood thanks @gabriel-tessier !
Have reverted the changes to the changelog 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants