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

Add collector component #930

Merged
merged 10 commits into from
Mar 28, 2022
Merged

Add collector component #930

merged 10 commits into from
Mar 28, 2022

Conversation

kickoke
Copy link
Contributor

@kickoke kickoke commented Mar 11, 2022

Problem

We wanted to reuse a MDX file that had a variable.
For some reasons (@tkatsoulas feel free to add them here) we weren't able to make it fully work.

After countless hours of trying to implement MDX correctly (😭 mishmash of Markdown, Javascript, and HTML), I simply resorted to writing a JS component. ...Yes, we 'lose' the benefit of writing markdown in this particular instance, but taking 5 mins to assign tags to style the output is way better than wasting 4 hours of making MDX work (RIP brain).

Solution

I have written a simple JSX function that takes an argument, in this case the collector, and passes it to the respective variables to create dynamic content.
I strongly suggest that for components that need a variable, we just resort to JS the next time.
For components that don't need to be adapted we can simply use markdown.

Closes #896

🎤 drop.

Todo:

  • Clean up old branches

@kickoke kickoke self-assigned this Mar 11, 2022
@netlify
Copy link

netlify bot commented Mar 11, 2022

Deploy Preview for netdata-docusaurus ready!

Name Link
🔨 Latest commit 238169a
🔍 Latest deploy log https://app.netlify.com/sites/netdata-docusaurus/deploys/6241f9353991620008fca809
😎 Deploy Preview https://deploy-preview-930--netdata-docusaurus.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@kickoke
Copy link
Contributor Author

kickoke commented Mar 11, 2022

I will structure the ActiveMQ doc according to the template and then we can merge this beautiful baby :)

@kickoke
Copy link
Contributor Author

kickoke commented Mar 11, 2022

Update: I removed the changes made to the ActiveMQ doc, because I realized that it will be overwritten by the ingest script anyways.

@ilyam8 and I will meet on Monday to discuss if we can use these components over in the netdata/go.d repo.

@kickoke
Copy link
Contributor Author

kickoke commented Mar 14, 2022

Short summary from the meeting:
Ilya is fine with our approach and supports our use of components! :)
We should include a note in files that contain components that will be rendered by Docusaurus and have an ingest rule that filters out that note for learn.

Copy link
Contributor Author

@kickoke kickoke left a comment

Choose a reason for hiding this comment

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

Merge the collector components all into a single file.

import React from 'react';
import CodeBlock from '@theme/CodeBlock';

export default function CollectorConfiguration({ configURL, moduleName }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename to : Enable collector

import React from 'react';
import CodeBlock from '@theme/CodeBlock';

export default function CollectorConfiguration({ configURL, moduleName }) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
export default function CollectorConfiguration({ configURL, moduleName }) {
export default function EnableCollector({ configURL, moduleName }) {

Delete old test file
@kickoke kickoke mentioned this pull request Mar 28, 2022
8 tasks
@kickoke kickoke merged commit 4e54097 into master Mar 28, 2022
@kickoke kickoke deleted the add-collector-component branch March 28, 2022 22:10
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.

2 participants