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

HACDOCS-975: Adding JFrog Artifactory plugin content #714

Conversation

missmesss
Copy link
Contributor

@missmesss missmesss commented Nov 15, 2024

After some discussion it was decided that this content belongs to the RHDH docs (Plugins docs), not the RHTAP docs that I work on.

The PR contains 2 files:

  • artifacts/rhdh-plugins-reference/jfrog-artifactory/jfrog-artifactory-plugin-admin.adoc is added to the Configuring dynamic plugins title
  • artifacts/rhdh-plugins-reference/jfrog-artifactory/jfrog-artifactory-plugin-user.adoc is added to the Using dynamic plugins title

Version(s): 1.3
Issue: HACDOCS-975

@rhdh-bot
Copy link
Collaborator

rhdh-bot commented Nov 15, 2024

Copy link

@karthikjeeyar karthikjeeyar left a comment

Choose a reason for hiding this comment

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

/lgtm

@missmesss
Copy link
Contributor Author

@Gerry-Forde Thank you for looking at this PR and providing labels. We've discussed things with Karthik, this PR is waiting for a QE review now (but I can't tag Jan Richter here).

@missmesss
Copy link
Contributor Author

@Gerry-Forde I have a couple of questions to you, when you have time, please.

Enabling VS Installing in the title
Which one should I keep? Most other plugin chapters are called “Installing and configuring”. At the same time the plugins are preinstalled with RHDH, and the procedures are about enabling them. There’s also a separate plugins installation guide. Might users get confused by that? Anyways, please, let me know if I should keep “Installing” in the title for historical and consistency reasons.

IDs
Do you use any IDs at the beginning of adocs? Some existing adocs begin with [[installation-and-configuration-tekton]], some with [id="installing-configuring-nexus-plugin"], some begin with a title.

-readmes
Some plugin repos have -readme docs alongside -admin and -user adocs. This PR has -admin and -user files only. Do I need to add a -readme file too? Readme files are included in titles/plugin-rhdh/title-plugin.adoc, but are they published anywhere?

Thank you in advance.

@Gerry-Forde
Copy link
Member

@Gerry-Forde I have a couple of questions to you, when you have time, please.

Enabling VS Installing in the title Which one should I keep? Most other plugin chapters are called “Installing and configuring”. At the same time the plugins are preinstalled with RHDH, and the procedures are about enabling them. There’s also a separate plugins installation guide. Might users get confused by that? Anyways, please, let me know if I should keep “Installing” in the title for historical and consistency reasons.

IDs Do you use any IDs at the beginning of adocs? Some existing adocs begin with [[installation-and-configuration-tekton]], some with [id="installing-configuring-nexus-plugin"], some begin with a title.

-readmes Some plugin repos have -readme docs alongside -admin and -user adocs. This PR has -admin and -user files only. Do I need to add a -readme file too? Readme files are included in titles/plugin-rhdh/title-plugin.adoc, but are they published anywhere?

Thank you in advance.

@missmesss
Enabling VS Installing in the title For consistency we use the Installing and configuring title which may be a little confusing as most plugins just need to be enabled. However, in future RHDH releases, plugins may in fact need to be installed if they are available in a separate repository/marketplace. The plugins installation guide that you refer to is currently being revised for RHDH 1.4, and may focus more on the installation of third-party plugins rather than pre-installed plugins.

IDs We should have IDs at the beginning of each document. Some of the content you see was originally sourced upstream and may use the anchor ([[]]) syntax for IDs. My preference is to use the the longhand (id=) syntax.

-readmes Again these are a legacy of sourcing upstream content, we do not use them in the RHDH docs and are only present for reference purposes (in fact, to avoid confusion,we should remove them once we have published content).

@missmesss
Copy link
Contributor Author

@jrichter1 and @Gerry-Forde Thank you so much for the feedback, I've added everything.
Sorry about the delay, the new RHTAP docs grabbed all of my attention. Released yesterday, yay.

@hmanwani-rh
Copy link
Member

@missmesss Please rebase this PR

@missmesss missmesss force-pushed the HACDOCS-975-jfrog-artifactory-plugin branch from 88dd635 to 5b6d099 Compare January 6, 2025 20:25
@missmesss
Copy link
Contributor Author

@karthikjeeyar @jrichter1 I hope we all enjoyed the holidays and the break and can now resolve the 1 remaining question remaining and close this PR :)

As per Jan's comment above, the source files don't need any changes if we're talking about the production RHDH instance.
So do app-config.yaml and packages/app/src/components/catalog/EntityPage.tsx need changes for the Artifactory plugin to work with DH? The plugin is a tech.preview, does it matter in this case?

Thanks!

@missmesss
Copy link
Contributor Author

@karthikjeeyar @jrichter1 The config step about modifying EntityPage.tsx has been removed now. Please, confirm this PR can be merged. Thank you.

The Configuration part is now 2 steps:

  1. Set the proxy in app-config.yaml
  2. Add annotation to catalog-info.yaml
    I hope it's correct now.

Copy link

@karthikjeeyar karthikjeeyar left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 14, 2025
Copy link

openshift-ci bot commented Jan 20, 2025

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Jan 20, 2025
@missmesss missmesss force-pushed the HACDOCS-975-jfrog-artifactory-plugin branch from 4c41402 to 37e8ca5 Compare January 20, 2025 17:58
@missmesss missmesss force-pushed the HACDOCS-975-jfrog-artifactory-plugin branch from 37e8ca5 to 6a0aca7 Compare January 20, 2025 18:05
@missmesss missmesss force-pushed the HACDOCS-975-jfrog-artifactory-plugin branch from 6a0aca7 to 548a034 Compare January 20, 2025 18:13
@missmesss
Copy link
Contributor Author

@Gerry-Forde If this PR is looking good to you, it can be merged.

The latest commit addresses the only change requested by Karthik (package path now includes the backstage-community repo) , and Jan has provided the lgtm too, so the SME and QE reviews are done.
Thank you.

@Gerry-Forde Gerry-Forde merged commit a682605 into redhat-developer:main Jan 21, 2025
3 checks passed
@Gerry-Forde
Copy link
Member

/cherrypick release-1.4

@openshift-cherrypick-robot
Copy link
Contributor

@Gerry-Forde: new pull request created: #866

In response to this:

/cherrypick release-1.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

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.

8 participants