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

Feat: Add path keyword #614

Merged
merged 6 commits into from
Jul 18, 2023
Merged

Feat: Add path keyword #614

merged 6 commits into from
Jul 18, 2023

Conversation

Meldiron
Copy link
Contributor

What does this PR do?

We use path as variable name inside function. Such parameter cannot be provided, otherwise there will be an error like

src/services/functions.ts:583:17 - error TS2300: Duplicate identifier 'path'.

583             let path = '/functions/{functionId}/executions'.replace('{functionId}', functionId);
                    ~~~~

Test Plan

  • QA with branch that needs path parameter

Related PRs and Issues

x

Have you read the Contributing Guidelines on issues?

Yes

Copy link
Member

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

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

@Meldiron Please add it to only relevant languages and also add it to abstract public function getIdentifierOverrides(): array; or the relevant language to give it a proper replacement like urlPath or similar.

@Meldiron Meldiron marked this pull request as draft March 7, 2023 16:14
@Meldiron
Copy link
Contributor Author

Meldiron commented Mar 7, 2023

Converting to draft. Will become relevant again near internal Functions G4 testing

@Meldiron
Copy link
Contributor Author

CleanShot 2023-05-11 at 22 30 06@2x

Should be good for now 😅

@Meldiron Meldiron marked this pull request as ready for review May 11, 2023 20:31
@lohanidamodar
Copy link
Member

@Meldiron can you also add this to identifier overrides so that we can have a override sucha as filePath or something instead of the default xpath when the keyword arises?
https://github.com/appwrite/sdk-generator/blob/3bd342fbfa771d573fbf877af8ec977e5c7e9194/src/SDK/Language/Dart.php#LL115C8-L115C8

Copy link
Member

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

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

this is good to merge, my last comment as I discussed with matej, as path can refer to file path, code path or something else, there is no proper identifier override that we can manually do so xpath is okay for now

@eldadfux eldadfux merged commit 0965751 into master Jul 18, 2023
@lohanidamodar lohanidamodar deleted the feat-add-path-keyword branch July 18, 2023 06:49
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.

4 participants