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 root path configuration #54

Merged
merged 1 commit into from
Sep 22, 2022

Conversation

eduardomourar
Copy link
Contributor

@eduardomourar eduardomourar commented Sep 17, 2022

Issue #, if available: N/A

Description of changes:

Allow users to customize the root path to be used by the Smithy VSCode extension. Otherwise, whenever we have the smithy-build.json file not in the workspace root, it fails with the following (as per commit 6e8f997):

Dump from .smithy.lsp.log

[18:14:14] Created a temporary folder for file contents /var/folders/dc/07t_8b2n41lgnz09y651104w0000gn/T/smithy-lsp7987430497366641088
[18:14:14] Recreating project from /smithy-vscode/test-fixtures/suite1
[18:14:14] Imports from config: [/smithy-vscode/test-fixtures/suite1] will be resolved against root /smithy-vscode/test-fixtures/suite1
[18:14:14] Discovered smithy files: [/smithy-vscode/test-fixtures/suite1/smithy/main.smithy]
[18:14:14] Downloading external dependencies for SmithyBuildExtensions(repositories = [], artifacts = [], imports = [])
[18:14:14] Downloaded external jars: []
[18:14:14] [ERROR] example.weather#GetCurrentTime: Unable to resolve trait aws.api#dataPlane. If this is a custom trait, then it must be defined before it can be used in a model. | Model /smithy-vscode/test-fixtures/suite1/smithy/main.smithy:18:1
[18:14:14] [ERROR] example.weather#Weather: Unable to resolve trait aws.api#service. If this is a custom trait, then it must be defined before it can be used in a model. | Model /smithy-vscode/test-fixtures/suite1/smithy/main.smithy:9:1
[18:14:14] Recreating project from /smithy-vscode/test-fixtures/suite1
[18:14:14] Imports from config: [/smithy-vscode/test-fixtures/suite1] will be resolved against root /smithy-vscode/test-fixtures/suite1
[18:14:14] Discovered smithy files: [/smithy-vscode/test-fixtures/suite1/smithy/main.smithy]
[18:14:14] Downloading external dependencies for SmithyBuildExtensions(repositories = [], artifacts = [], imports = [])
[18:14:14] Downloaded external jars: []
[18:14:14] [ERROR] example.weather#GetCurrentTime: Unable to resolve trait aws.api#dataPlane. If this is a custom trait, then it must be defined before it can be used in a model. | Model /smithy-vscode/test-fixtures/suite1/smithy/main.smithy:18:1
[18:14:14] [ERROR] example.weather#Weather: Unable to resolve trait aws.api#service. If this is a custom trait, then it must be defined before it can be used in a model. | Model /smithy-vscode/test-fixtures/suite1/smithy/main.smithy:9:1
[18:14:14] [ERROR] example.weather#GetCurrentTime: Unable to resolve trait aws.api#dataPlane. If this is a custom trait, then it must be defined before it can be used in a model. | Model /smithy-vscode/test-fixtures/suite1/smithy/main.smithy:18:1
[18:14:14] [ERROR] example.weather#Weather: Unable to resolve trait aws.api#service. If this is a custom trait, then it must be defined before it can be used in a model. | Model /smithy-vscode/test-fixtures/suite1/smithy/main.smithy:9:1
[18:14:14] Recompiling /smithy-vscode/test-fixtures/suite1/smithy/main.smithy (with temporary content Optional.empty) raised 2 diagnostics

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@eduardomourar eduardomourar marked this pull request as ready for review September 17, 2022 16:20
@eduardomourar eduardomourar requested a review from a team as a code owner September 17, 2022 16:20
src/extension.ts Outdated Show resolved Hide resolved
test-fixtures/suite3/smithy-build.json Outdated Show resolved Hide resolved
test-fixtures/suite2/smithy-build.json Outdated Show resolved Hide resolved
test-fixtures/suite1/smithy/smithy-build.json Outdated Show resolved Hide resolved
test-fixtures/suite1/.vscode/settings.json Outdated Show resolved Hide resolved
src/extension.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@@ -1,6 +1,6 @@
{
"maven": {
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.23.1"],
"dependencies": ["software.amazon.smithy:smithy-aws-traits:1.25.0", "software.amazon.smithy:smithy-waiters:1.25.0"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does smithy-waiters need to be pulled in for this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to ensure we were testing minimally 2 packages being installed.

@milesziemer
Copy link
Contributor

Thank you for separating the PRs, they have been merged. Please rebase and push this branch back up to avoid conflict.

@milesziemer milesziemer merged commit d11e728 into smithy-lang:main Sep 22, 2022
@eduardomourar eduardomourar deleted the feat/root-path-config branch September 22, 2022 20:22
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