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

Make collectors/COLLECTORS.md have its list autogenerated from integrations.js #15995

Merged
merged 7 commits into from Sep 28, 2023

Conversation

Ancairon
Copy link
Member

@Ancairon Ancairon commented Sep 18, 2023

Summary

This script reads the integrations/integrations.js file and generates the list of data collection integrations inside collectors/COLLECTORS.md, with proper links that Learn can replace into Learn links.

Due to the string replacements the script does, I have ran it once in this PR, to change the top header (that it looks to split every time) to the desired one.

@Ferroin please check the script and the action addition
@sashwathn please check the outcome in the markdown file

@Ancairon Ancairon self-assigned this Sep 18, 2023
@github-actions github-actions bot added area/ci area/docs area/collectors Everything related to data collection area/metadata Integrations metadata labels Sep 18, 2023
Copy link
Member

@Ferroin Ferroin left a comment

Choose a reason for hiding this comment

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

Workflow looks good. Left some suggestions for the Python code.

integrations/gen_doc_collector_page.py Outdated Show resolved Hide resolved
integrations/gen_doc_collector_page.py Outdated Show resolved Hide resolved
integrations/gen_doc_collector_page.py Outdated Show resolved Hide resolved
integrations/gen_doc_collector_page.py Show resolved Hide resolved
integrations/gen_doc_collector_page.py Outdated Show resolved Hide resolved
integrations/gen_doc_collector_page.py Outdated Show resolved Hide resolved
Ferroin
Ferroin previously approved these changes Sep 19, 2023
sashwathn
sashwathn previously approved these changes Sep 20, 2023
Copy link
Member

@ilyam8 ilyam8 left a comment

Choose a reason for hiding this comment

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

I think we should add it to the integration workflow. I find it weird to merge 2 PRs instead of 1 when they are totally related.

@Ancairon
Copy link
Member Author

Well, they are not entirely related, one generates the file and the other is one piece that utilizes the js file.

Like one is making something that is used by everyone, while the other is responsible for generating documentation out of it 🤔

@Ferroin
Copy link
Member

Ferroin commented Sep 20, 2023

I would tend to agree with Ilya on having it be one PR instead of more than one.

We can simply add a step to the existing workflow to run this script after the new integrations.js file is generated.

@ilyam8
Copy link
Member

ilyam8 commented Sep 20, 2023

one generates the file and the other is one piece that utilizes the js file

one generates the file and
the other uses this file to generate docs

not entirely related

wait what

1st always triggers 2nd, I think those changes should be in one PR.

@Ancairon
Copy link
Member Author

Thanks for the feedback, I added the steps from the action file I had made to the existing action for integrations

ilyam8
ilyam8 previously approved these changes Sep 25, 2023
@Ancairon
Copy link
Member Author

found a small bug, gonna push a new commit to fix and re-request approvals, parentheses in the links should not be there, but simple fix

@Ancairon Ancairon merged commit 5ff2ec1 into master Sep 28, 2023
142 of 143 checks passed
@ilyam8 ilyam8 deleted the collectors-page-generation branch September 28, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci area/collectors Everything related to data collection area/docs area/metadata Integrations metadata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants