Skip to content

Conversation

@jackwilsdon
Copy link
Contributor

What issue type does this pull request address? (keep at least one, remove the others)
/kind documentation

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
Moves is_dependency from the "Global Functions" section of the built-in functions list to the "Pipeline-Only Functions" section (is_dependency is pipeline-only).

Please provide a short message that should be published in the DevSpace release notes
Updated classification of is_dependency in documentation.

@netlify
Copy link

netlify bot commented Nov 23, 2023

Deploy Preview for devspace-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 926b3e2
🔍 Latest deploy log https://app.netlify.com/sites/devspace-docs/deploys/656db055117b6e0008fb433f
😎 Deploy Preview https://deploy-preview-2761--devspace-docs.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.

Copy link
Collaborator

@lizardruss lizardruss left a comment

Choose a reason for hiding this comment

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

@jackwilsdon

Thanks for the PR!

It looks like there are some issues with our function partials generation script. The first being that it's not included in gendocs.sh. The second is that it's not placing these in the correct category.

If you get a chance to look into those issues, that would be great! Otherwise I can take a look soon.

@jackwilsdon
Copy link
Contributor Author

jackwilsdon commented Nov 30, 2023

I've tried running the partials generation script but it generates a whole heap of changes - they do seem correct but should they be included in this PR? For example:

diff --git a/docs/pages/configuration/_partials/functions/is_empty.mdx b/docs/pages/configuration/_partials/functions/is_empty.mdx
index a2fadf770..ff55ba29b 100755
--- a/docs/pages/configuration/_partials/functions/is_empty.mdx
+++ b/docs/pages/configuration/_partials/functions/is_empty.mdx
@@ -3,9 +3,9 @@
 <details className="config-field -function" data-expandable="false">
 <summary>
 
-### `is_empty` <span className="config-field-type">[value]</span> <span className="config-field-enum"></span> <span className="config-field-default -return">bool</span> <span className="config-field-required" data-required="false">pipeline only</span>  {#is_empty}
+### `is_empty` <span className="config-field-type">[value]</span> <span className="config-field-enum"></span> <span className="config-field-default -return">int</span> <span className="config-field-required" data-required="false">pipeline only</span>  {#is_empty}
 
-Returns true if the value of the argument is empty string
+Returns exit code 0 if the value of the argument is empty string
 
 </summary>
 

is_in also seems to be completely missing from the documentation generator. I'm happy to add this in either in this PR or a separate one.

@lizardruss
Copy link
Collaborator

@jackwilsdon Thank you so much! Yeah I think including it in this PR would be good. Adding is_in should be pretty straightforward. I'm just learning that the text for that documentation is here, and looks like it was updated but the partial generation was never run afterwards. Looks like is_dependency can be moved to the correct group by removing this flag.

You've probably figured all this out, but I'm also just learning about this part of the document generation code 👍

Signed-off-by: Jack Wilsdon <jack+github@wilsdon.me>
Signed-off-by: Jack Wilsdon <jack+github@wilsdon.me>
is_dependency can only be called from inside pipelines.

Signed-off-by: Jack Wilsdon <jack+github@wilsdon.me>
Signed-off-by: Jack Wilsdon <jack+github@wilsdon.me>
Signed-off-by: Jack Wilsdon <jack+github@wilsdon.me>
@jackwilsdon
Copy link
Contributor Author

I've updated this PR to include the changes discussed 👍

Copy link
Collaborator

@FabianKramm FabianKramm left a comment

Choose a reason for hiding this comment

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

@jackwilsdon thanks a lot for the PR! LGTM!

@lizardruss lizardruss merged commit e93bd85 into devspace-sh:main Feb 8, 2024
@jackwilsdon jackwilsdon deleted the move-is-dependency branch February 8, 2024 22:55
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.

3 participants