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: new dependency graph #117

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mateuszjenek
Copy link
Contributor

@mateuszjenek mateuszjenek commented Nov 20, 2024

Replace old graph library with the new Humanitec Resource Graph component

Result:
Zrzut ekranu 2024-12-2 o 14 43 06

@mateuszjenek mateuszjenek force-pushed the new_dependency_graph branch 6 times, most recently from 52b40ad to b8a78a4 Compare November 21, 2024 12:34
@mateuszjenek mateuszjenek force-pushed the new_dependency_graph branch 3 times, most recently from 002954b to 4f5faec Compare December 2, 2024 13:48
@mateuszjenek mateuszjenek marked this pull request as ready for review December 2, 2024 13:48
@mathieu-benoit
Copy link

mathieu-benoit commented Dec 5, 2024

@mateuszjenek, anything missing here to merge this and publish a new version? That's awesome!

} else if (result.stderr.includes('environment is required')) {
} else if (
result.stderr.includes('environment is required') ||
result.stderr.includes('missing env')
Copy link
Member

Choose a reason for hiding this comment

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

Where are these new errors coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CLI returns unconsistent error messages and I found out that in most places it returns: environment is required, but in some: missing env

Comment on lines 22 to 30
const deploymentUrl =
'/orgs/' +
organizationId +
'/apps/' +
applicationId +
'/envs/' +
environmentId +
'/deploys/' +
deployId;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const deploymentUrl =
'/orgs/' +
organizationId +
'/apps/' +
applicationId +
'/envs/' +
environmentId +
'/deploys/' +
deployId;
const deploymentUrl = `/orgs/${organizationId}/apps/${applicationId}/envs/${environmentId}/deploys/${deployId}`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice suggestion, applied in all places!

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