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

Alternatives to committing #34

Merged
merged 10 commits into from
Aug 24, 2021
Merged

Conversation

ageorgou
Copy link
Contributor

@ageorgou ageorgou commented Aug 22, 2021

This is meant to address #16.

  • Add an input option push should_push that lets you choose not to commit and push the diagram (true by default, to maintain current behaviour).
  • As suggested in one of the comments, add an option to upload the diagram as an artifact of the workflow (not done by default).
  • Also allow getting the SVG directly as an output of the action. This could be useful, for example, if you want to programmatically comment on a PR with the markdown of the diagram.

Note sure if this is overkill, comments very welcome.

Copy link
Contributor

@Wattenberger Wattenberger 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 wonderful, thanks for the PR! I had no idea about artifacts, but they seem like a really flexible way to prevent from pushing every change

README.md Outdated

The maximum number of nested folders to show files within. A higher number will take longer to render.

Default: 9

## `commit_message`
### `push`
Copy link
Contributor

Choose a reason for hiding this comment

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

do you mind renaming this to should_push, just for extra clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

README.md Outdated

Should be a boolean value, i.e. `true` or `false`. See `commit_message` and `branch` for how to customise the commit.

Default: `false`
Copy link
Contributor

@Wattenberger Wattenberger Aug 23, 2021

Choose a reason for hiding this comment

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

we should probably leave this as true, for backwards compatibility, like you mentioned in your PR. I know there are projects using repo-visualizer@main

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, the default was indeed true but I miswrote it in the README! Will fix it.

@Wattenberger
Copy link
Contributor

wonderful! thanks for the PR and quick updates! this is a great feature!

@Wattenberger Wattenberger merged commit ccc127b into githubocto:main Aug 24, 2021
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