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

Documentation for component integration. #77

Merged
merged 31 commits into from Mar 29, 2023

Conversation

shinde-rahul
Copy link
Contributor

@shinde-rahul shinde-rahul commented Oct 17, 2022

This fixes, #62
This PR adds,

  • Documentation for plugin configuration.
  • Documentation for ConfigFormNotesInterface

@RCheesley
Copy link
Sponsor Member

Looks like you have a broken link to fix on this one @shinde-rahul - the build linter is failing.

The repo https://github.com/mautic/plugin-integrations does not exist.

There are also a lot of RST errors where duplicated headings are being used, and Vale errors too. Happy to walk thought it with you!

Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Thanks for getting started with bringing this over @shinde-rahul, awesome job!

There are a few things to bear in mind with the docs which we need to write up:

  • Links need to be created with the external links extension markup
  • We don't use Camel Case in headings - only capitalise within the heading if you are referring to a feature of Mautic (eg Contacts, Companies, Plugins etc)
  • Always use example.com as the domain name when referring to a URL - with Mautic I think we also append /api for the API but please do check that.

You might find it easier to review the PR in GitPod if you are not getting the feedback about the RST / Vale issues in your IDE. Give me a shout if you need any help with any of this!

docs/plugins/integrations/authentication.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/authentication.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/authentication.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/authentication.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/authentication.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/sync.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/sync.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/sync.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/sync.rst Outdated Show resolved Hide resolved
docs/plugins/integrations/sync.rst Outdated Show resolved Hide resolved
shinde-rahul and others added 6 commits October 18, 2022 18:50
Co-authored-by: Ruth Cheesley <ruth.cheesley@acquia.com>
Apply suggestions from the code review

Co-authored-by: Ruth Cheesley <ruth.cheesley@acquia.com>
@RCheesley
Copy link
Sponsor Member

Is this the same as #76? Or something new?

@shinde-rahul
Copy link
Contributor Author

Is this the same as #76? Or something new?

@RCheesley, #76 does not hold changes for the ConfigFormNotesInterface as this new feature is introduced in 5.x.
I was not sure, so I raised for both versions.

@shinde-rahul shinde-rahul changed the title Documentation for plugins configuration. Documentation for component integration. Nov 2, 2022
@RCheesley
Copy link
Sponsor Member

@shinde-rahul could you review/update the outstanding unresolved conversations please? Thanks!

@RCheesley
Copy link
Sponsor Member

Hi @shinde-rahul do you think you can get this finished up?

@shinde-rahul
Copy link
Contributor Author

@RCheesley, this is stuck with either

How to fix this, I need help here.

@RCheesley
Copy link
Sponsor Member

Where is the problem in the code?

You can usually wrap it with this:

.. vale off

Text here

.. vale on

@shinde-rahul
Copy link
Contributor Author

shinde-rahul commented Jan 25, 2023

@RCheesley, thanks! I forgot about the vale on/off. This is now ready for review.

Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

This looks good to me, would value a developer review of the content before we merge. Thanks for the PR @shinde-rahul 🚀

Here's how to review with GitPod for folks who might not have done that before on the new docs: https://watch.screencastify.com/v/Ttn3Vr7MQbHkefkYUeJe

Shout if you need any help!

@RCheesley RCheesley requested a review from escopecz March 29, 2023 09:26
Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks Rahul!

@escopecz escopecz merged commit 73f124d into mautic:5.x Mar 29, 2023
2 checks passed
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.

None yet

4 participants