Skip to content

Comments

[BE-4789]: Initially fixed for wf step templates#731

Merged
ftmzhrsckldr merged 11 commits intodevelopfrom
fix/BE-4789
Dec 5, 2024
Merged

[BE-4789]: Initially fixed for wf step templates#731
ftmzhrsckldr merged 11 commits intodevelopfrom
fix/BE-4789

Conversation

@boztopuz
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Nov 12, 2024

Deploy Preview for reverent-galileo-8ef035 ready!

Name Link
🔨 Latest commit 2abb940
🔍 Latest deploy log https://app.netlify.com/sites/reverent-galileo-8ef035/deploys/67515225e85eff000896f57a
😎 Deploy Preview https://deploy-preview-731--reverent-galileo-8ef035.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 configuration.

@boztopuz boztopuz self-assigned this Nov 14, 2024
@boztopuz
Copy link
Contributor Author

boztopuz commented Nov 14, 2024

@ftmzhrsckldr I updated all WF step docs to have the same template.The changes were applied to each step so that the templates of all step documents are the same. Please check here before you start the review.

Looking at these before you start will help us to spot any logical errors or mistakes early on and avoid wasting time. Because most of the changes are the same and repetitive.

What has changed?

1- All required Prequisites introduction sentences have been changed as follows.
Before running the **<Step Name>** step, you must complete certain prerequisites, as detailed in the table below:

2- Corrected the formatting of all info, caution, danger etc boxes in the .md file.
3- Added missing $ signs in all input tables.
4- Removed $ sign in all output tables.
5- Removed output section in docs for the steps that has not output variables.
6- All sensitive variable danger boxes have been replaced with this.

danger Sensitive Variables

Please do not use sensitive variables such as **Username**, **Password**, **API Key**, or **Personal Access Key** directly within the step.

We recommend using [**Environment Variables**](/environment-variables/managing-variables) groups for such sensitive variables.

7- Duplicated post processor step doc has been deleted.
8- All FAQ headings have been edited.
9- Replaced the introductory sentences of all input headers with this.
This step contains some input variable(s). It needs these variable(s) to work. The table below gives explanation for this variable(s).

10- Replaced the introductory sentences of all output headers with this one.
The output(s) resulting from the operation of this component are as follows:

11- The introductory sentence under the prerequisites heading for steps without prerequisites has been replaced with the following.
There are no prerequisites required before using the **<Step Name>** step.
12- Added missing prerequisites introductory sentence.

Note: I have not change anything in Saucectl doc. Because we already have new version of it in this PR.

@boztopuz boztopuz marked this pull request as ready for review November 14, 2024 11:12
@boztopuz boztopuz changed the title Initially fixed for wf step templates [BE-4789]: Initially fixed for wf step templates Nov 14, 2024
@ftmzhrsckldr
Copy link
Contributor

@ftmzhrsckldr I updated all WF step docs to have the same template.The changes were applied to each step so that the templates of all step documents are the same. Please check here before you start the review.

Looking at these before you start will help us to spot any logical errors or mistakes early on and avoid wasting time. Because most of the changes are the same and repetitive.

What has changed?

1- All required Prequisites introduction sentences have been changed as follows. Before running the **<Step Name>** step, you must complete certain prerequisites, as detailed in the table below:

2- Corrected the formatting of all info, caution, danger etc boxes in the .md file. 3- Added missing $ signs in all input tables. 4- Removed $ sign in all output tables. 5- Removed output section in docs for the steps that has not output variables. 6- All sensitive variable danger boxes have been replaced with this.

danger Sensitive Variables

Please do not use sensitive variables such as **Username**, **Password**, **API Key**, or **Personal Access Key** directly within the step.

We recommend using [**Environment Variables**](/environment-variables/managing-variables) groups for such sensitive variables.

7- Duplicated post processor step doc has been deleted. 8- All FAQ headings have been edited. 9- Replaced the introductory sentences of all input headers with this. This step contains some input variable(s). It needs these variable(s) to work. The table below gives explanation for this variable(s).

10- Replaced the introductory sentences of all output headers with this one. The output(s) resulting from the operation of this component are as follows:

11- The introductory sentence under the prerequisites heading for steps without prerequisites has been replaced with the following. There are no prerequisites required before using the **<Step Name>** step. 12- Added missing prerequisites introductory sentence.

Note: I have not change anything in Saucectl doc. Because we already have new version of it in this PR.

@boztopuz I did not see any problems with what was commented. However, after reviewing the documents, if one of the templates we used does not match that document, I can add feedback. For example, if the warn message you gave in step 6 does not match the variable that should be secret in the doc, I can add feedback. Other than that, I did not see any problems.

By the way, there were errors in document linking in old documents. Instead of internal linking, there were links starting with "http...". Were you able to check these as well?

@boztopuz
Copy link
Contributor Author

boztopuz commented Nov 15, 2024

By the way, there were errors in document linking in old documents. Instead of internal linking, there were links starting with "http...". Were you able to check these as well?

@ftmzhrsckldr Unfortunately, I did not check the links which include http...., I will try to check as soon as possible all of them today.

@ftmzhrsckldr
Copy link
Contributor

ftmzhrsckldr commented Nov 21, 2024

@boztopuz I will make a checklist of the list you shared. To make sure I have changed the necessary points in each document. While doing this, I have a few questions:

2- Corrected the formatting of all info, caution, danger etc boxes in the .md file.

What do you mean by formatting?

5- Removed output section in docs for the steps that has not output variables.

There are documents where I added the ##Output Variable heading in some component and indicated that there was no output but what has changed as the component output. I don't think these should be deleted. What do you think?

8- All FAQ headings have been edited.

Is this what you mean by that?

## Custom Script FAQ => ## FAQ

Is there anything else I should check other than that?

@boztopuz
Copy link
Contributor Author

boztopuz commented Nov 21, 2024

@ftmzhrsckldr

Corrected the formatting of all info, caution, danger etc boxes in the .md file.

Old version of document danger info etc boxes are like this in md files

:::danger
danger details
:::

I changed all of these like

:::danger

danger details

:::

The reason I do this is that some errors can occur in the docs when using non-space format. We have experienced this in the past

Removed output section in docs for the steps that has not output variables.

When you look at the documents, you will see that I did not delete all of them, I deleted only the sections that do not have a table and a specific description.

All FAQ headings have been edited.

Yes that is true, I changed the faq sections like

## Custom Script FAQ => ## FAQ

@ftmzhrsckldr
Copy link
Contributor

Feedback Summary

@boztopuz The initial documents were very well-prepared and required minimal feedback. However, as we progressed, the amount of feedback increased. I believe having too many changes in a single PR might have reduced focus.

I structured my feedback so that you can directly apply the suggestions if you approve them. However, for some unchanged paragraphs, GitHub did not allow me to add feedback. In those cases, I made corrections by linking them to the nearest editable line.

Feedback Summary

  1. Many links were correctly updated, but some were missed in certain documents (e.g., links still starting with "http"). Since some untouched files (like index.md) contained these links, I fixed them with this commit: 143cb83. Feel free to review it if needed.
  2. Some issues in untouched index.md files were not fixed since I could not leave feedback on unchanged lines. The following should be reviewed and corrected:
    • Add $ to environment variables where applicable.
    • Separate ::: blocks from content.
  3. A few points mentioned here were missed in some documents:
    • In earlier documents, reserved environment variables started with $, but this was inconsistent in later documents.
    • Some ::: blocks were not separated from content.
    • The "Sensitive Variables" warning was missing in one document, though it was correct in others.
  4. Metadata tags: Many were incomplete or copied from other documents without proper editing.
  5. Metadata descriptions: Some contained unnecessary details (e.g., prerequisites).
  6. Inconsistent formatting within the same document:
    • Some step names were bold, while others were not.
    • File names were sometimes in backticks (``) but plain in other cases.
  7. Some typos were missed, and I have corrected them.
  8. A few documents did not follow the common pattern for source code links and descriptions. I fixed these.
  9. FAQ headings were corrected, but the content was not fully reviewed, leading to additional fe

CC: @csonuryilmaz

@boztopuz boztopuz requested a review from ftmzhrsckldr December 2, 2024 15:51
@ftmzhrsckldr
Copy link
Contributor

The vast majority of feedback has been implemented correctly.

@boztopuz 3 of the feedbacks I gave are new problems that occur after the update. The remaining 5 are deficiencies that I noticed only now but did not notice in my previous feedback.

Note: One of my previous feedback I marked unresolved. FYI

@boztopuz boztopuz requested a review from ftmzhrsckldr December 5, 2024 07:11
@github-actions github-actions bot added the autoupdate Useful when you want to autoupdate your branch when develop branch is updated label Dec 5, 2024
@ftmzhrsckldr ftmzhrsckldr merged commit 814b9ff into develop Dec 5, 2024
@csonuryilmaz csonuryilmaz deleted the fix/BE-4789 branch December 5, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autoupdate Useful when you want to autoupdate your branch when develop branch is updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants