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

[Enhancement] No CI system / local output option #46

Closed
HenrikStanley opened this issue Aug 10, 2022 · 7 comments · Fixed by #49
Closed

[Enhancement] No CI system / local output option #46

HenrikStanley opened this issue Aug 10, 2022 · 7 comments · Fixed by #49
Assignees
Labels
enhancement New feature or request released

Comments

@HenrikStanley
Copy link

From any CI/CD tool, it is possible to push to a Pull Request from the Github CLI.
For example from inside GitHub actions, anything we would like to post that has been output as a file from another task could be output like this:

    - name: Comment on Pull Request
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      run: |
        gh pr comment --body-file ${{ steps.diff.outputs.comment_file }}

My suggestion is to add a NO CI option that does not attempt to post the output, but simple supplies the output file with the markdown in it.

Other systems can then easily just take that output and post it themselves by handing it over without having to worry about specific CI system details.

This would make the tool more versatile and allow for support of other systems without implementing a ton of API details.

@karlderkaefer karlderkaefer self-assigned this Aug 11, 2022
@karlderkaefer
Copy link
Owner

karlderkaefer commented Aug 11, 2022

this is a brilliant idea. That could have saved tons of code. It would just miss some logic to update or delete existing comments, Looking into this next days

@HenrikStanley
Copy link
Author

Specifically for GitHub, the CLI has a great many options for manipulating a PR:
https://cli.github.com/manual/gh_pr

It comes pre-installed in the Github CI Runners, but would obviously need to be installed into the setup another CI system is used but GitHub PR / Git system.

For a more modular / Unix tool type setup, an idea would be have a tool do one thing and do it will.
For example cdk-notifier could be a tool which main purpose is to convert cdk diff to a nice markdown that can be posted in to PRs for various git systems.

Then, using either the native tools for that git system, such as the GH cli, the output of cdk-notifier is simply used as an input and posted.

This is obviously a different philosophy where you pipe input and output between different tools, but it does allow for great flexibility.

If you then have a system you wish to integrate details from, that does not have a CLI tool that gives you what you want, the idea would be to create a new CLI tool for that purpose. Keeping each code base lean and with a more specific purpose.

These are all just ideas about how the tool could be designed, in the end it comes down to the design goals of the tool :)
Coming from a Unix / Linux background myself, I just have a strong preference for tools that does one thing and do them well as it makes it very modular and easy to combine them with other tools to get great results.

(Also kudos on the cool project)

@karlderkaefer
Copy link
Owner

karlderkaefer commented Aug 20, 2022

I like your philosophy, to take over single-responsibility-principal not only for code but also tools. It provide much better flexibility when it comes to chaining tools. It may change the required complexity to manage orchestration, or better to say to leave it up to the user. However changing direction is too late for this project. Do not want to throw away the work that has been done 😛 I mean there is some kind of separation as the client implementations for github and gitlab are from external repositories. From beginning the logic how create, update and delete comments was part of the design for better user experience.

Anyway your feature request totally makes sense and brings more value to the tool, I will add this as soon as I can

@karlderkaefer karlderkaefer added the enhancement New feature or request label Aug 20, 2022
karlderkaefer added a commit that referenced this issue Aug 20, 2022
@karlderkaefer
Copy link
Owner

karlderkaefer commented Aug 20, 2022

@HenrikStanley do you want the markdown diff as stdout and/or as file? I assume both options can be useful, what do you think? any suggestions for parameter name and description?

@HenrikStanley
Copy link
Author

@HenrikStanley do you want the markdown diff as stdout and/or as file? I assume both options can be useful, what do you think? any suggestions for parameter name and description?

There are a few ways to go about this, I think the "simplest" one might be to simply allow the application to output to multiple sources at the same time depending on the feature flags.

So first up would be adding to the --ci flag with a disable option.
--ci string CI System used [circleci|bitbucket|none]

Replace none with false, disable or which ever word you prefer the most.

Then add two new flags.
--output-file
and
--output-stdout

--output-file can take a path.
--output-stdout is a bool.

With this setup, a user could select to print to the CI system and stdout at the same time which would make debugging easier.
Or only output to a file.

This at least would be my suggestion.

karlderkaefer pushed a commit that referenced this issue Dec 10, 2022
# [2.3.0](v2.2.3...v2.3.0) (2022-12-10)

### Bug Fixes

* add more test for transformer ([476b438](476b438))
* do not validate config when running no post mode ([199524d](199524d))
* use boolean for delete arg ([b74c0ad](b74c0ad)), closes [#47](#47)

### Features

* add no post mode ([dcb416d](dcb416d)), closes [#46](#46)
@karlderkaefer
Copy link
Owner

🎉 This issue has been resolved in version 2.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@karlderkaefer
Copy link
Owner

@HenrikStanley I have added the feature --no-post-mode however in a minimal version. Since the cdk log file must exists, I will just additionally add .diff extension. It will write to stdout and file. https://github.com/karlderkaefer/cdk-notifier/blob/main/README.md#no-post-mode

So this contains not all the features you asked for, because I do not want bloat the command line arguments and keep it simple. I would require more refactoring by splitting cli commands. But I hope you can work with this version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants