Skip to content

[FEATURE] Load data from composer.json if present #802

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

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

linawolf
Copy link
Contributor

@linawolf linawolf commented Jan 4, 2024

Load data like description, support links, composer name etc from the composer.json if one is found.

@linawolf linawolf requested a review from jaapio January 4, 2024 17:18
@linawolf linawolf self-assigned this Jan 4, 2024
Copy link
Member

@jaapio jaapio left a comment

Choose a reason for hiding this comment

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

I would opt for a more generic solution for this feature. If we would pass the projectNode in an event, we could attach any listener to the project node to add extra variables.

By doing this the run command doesn't have to be aware of the listeners that are part of building step of the project node. And there would be no need to change the Settings as they are just used to transfer the data to the project node?

If there is a reason to have the composer information in the settings, I would suggest to create a more generic format to store them in the settings. Right now the settings object is really specify changed for the composer addition. And if we have more of these kind of topic it would be grow fast.

@linawolf
Copy link
Contributor Author

linawolf commented Jan 6, 2024

How, when and where would you pass the project node?

@linawolf
Copy link
Contributor Author

@jaapio how can we proceed?

@jaapio
Copy link
Member

jaapio commented Jan 12, 2024

How, when and where would you pass the project node?

I would trigger in event just like you do right now in the run command. Something like ProjectLoaded which includes the project node.

@linawolf linawolf force-pushed the feature/load-composer-json branch 2 times, most recently from 68358a8 to ce6fe82 Compare January 13, 2024 09:18
@linawolf
Copy link
Contributor Author

@jaapio please have another look if this is what you intended

@jaapio jaapio enabled auto-merge January 14, 2024 15:55
@jaapio
Copy link
Member

jaapio commented Jan 14, 2024

@linawolf can you please have a look at the merge conflicts?

@linawolf linawolf force-pushed the feature/load-composer-json branch from ce6fe82 to 62aa76a Compare January 14, 2024 16:10
Load data like description, support links, composer name etc
from the composer.json if one is found.
@linawolf linawolf force-pushed the feature/load-composer-json branch from 62aa76a to df864d0 Compare January 14, 2024 16:11
@jaapio jaapio merged commit a9f3305 into main Jan 14, 2024
@jaapio jaapio deleted the feature/load-composer-json branch January 14, 2024 16:18
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