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

refactor: CircleCI config removed #246

Closed

Conversation

Pradhvan
Copy link
Member

Removed Circle CI config for changelog management

closes #139

@Pradhvan Pradhvan requested review from a team as code owners July 29, 2020 17:02
@Pradhvan Pradhvan force-pushed the MoveChangeLogToGithubActions branch from a905956 to 23792c3 Compare July 29, 2020 17:10
@Pradhvan Pradhvan force-pushed the MoveChangeLogToGithubActions branch 2 times, most recently from 3137524 to 951ec61 Compare July 29, 2020 19:58
- name: Changelog Tool
uses: rcmachado/changelog-action@v1
with:
args: init --output CHANGELOG.md --compare-url https://keepachangelog.com/en/1.0.0/
Copy link
Member Author

Choose a reason for hiding this comment

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

@camilamaia is this the correct way of using the Changelog Tool or should I keep this in the lint.yml

Copy link
Member

Choose a reason for hiding this comment

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

@Pradhvan I believe the command can be the same we are already using: https://github.com/scanapi/scanapi/blob/master/.circleci/config.yml#L17

From what I understood, init would create a new file, but here we only want to check the lint, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@camilamaia You're correct - init will initialize a new file. But as there is no lint command yet, the diff command should be used instead (there is an issue open to implement it: rcmachado/changelog#71).

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried the diff[0] but it's not working. I you can take a look @rcmachado it would be a great help.

[0]https://github.com/scanapi/scanapi/pull/246/checks?check_run_id=932150102

@camilamaia
Copy link
Member

@rcmachado would you mind to giving us some help reviewing this PR?

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Removed
- APIKeyMissingError [#218](https://github.com/scanapi/scanapi/pull/218)
- Move Changelog Lint from CircleCI to GitHub Actions [#139](https://github.com/scanapi/scanapi/issues/139)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be in the Changed section

Copy link
Member Author

Choose a reason for hiding this comment

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

@vinigfer yes will move it. 😄

Copy link
Member

@vinigfer vinigfer left a comment

Choose a reason for hiding this comment

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

LGTM, but we need to get te diff argument working in the action

@vinigfer vinigfer changed the title CircleCI config removed refactor: CircleCI config removed Aug 1, 2020
@Pradhvan Pradhvan force-pushed the MoveChangeLogToGithubActions branch from a44b724 to 65e9331 Compare August 2, 2020 11:30
- name: Changelog Tool
uses: rcmachado/changelog-action@v1
with:
args: diff -u <(changelog fmt -f CHANGELOG.md) CHANGELOG.md
Copy link
Member

@barbosa barbosa Aug 2, 2020

Choose a reason for hiding this comment

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

@Pradhvan the problem here is because args is what is being passed as the argument to the changelog command. This diff you want is system's. The way it is implemented here, GitHub Actions is going to call:

$ changelog diff -u <(changelog fmt -f CHANGELOG.md) CHANGELOG.md 

... resulting in an error given that the tool has no argument called diff.

There are a few ways to solve this:

  1. Implement the diff command on @rcmachado 's tool (or as he points out in Add --check option to fmt rcmachado/changelog#71: fmt --check)
  2. Do not use his GitHub Action and simply download and execute the command yourself
  3. Keep using his GitHub Action but try to play with shell arguments to execute fmt, get the results from it, and then call system's diff

IMO:

  • 1 is the best option if you know (or want to play with) Golang.
  • 3 might work well if you know how to use xargs (I don't 😅)
  • 2 could be achieved by some sort of the following commands:
- name: Changelog Tool
- run: |
  go get github.com/rcmachado/changelog
  diff -u <(./bin/changelog fmt -f CHANGELOG.md) CHANGELOG.md

Copy link
Member Author

Choose a reason for hiding this comment

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

@barbosa fun thing is I actually picked up Go and will go for 1 😄

@camilamaia camilamaia force-pushed the master branch 5 times, most recently from 3a3c742 to 35724ec Compare October 8, 2020 11:48
Fix Publish to Test PyPI Action
Co-authored-by: Flavio Sales Truzzi <[email protected]>
@Pradhvan
Copy link
Member Author

I might not be able to give time on this. So closing the PR.

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.

Move Changelog Lint from CircleCI to GitHub Actions
7 participants