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

Update README.md #378

Merged
merged 2 commits into from
May 14, 2020
Merged

Update README.md #378

merged 2 commits into from
May 14, 2020

Conversation

reckerswartz
Copy link
Contributor

GitHub Actions Integration updated according to the latest working case.

 GitHub Actions Integration updated according to the latest working case.
@reckerswartz reckerswartz requested a review from a team as a code owner May 12, 2020 01:12
README.md Outdated
- run: gem install pronto pronto-rubocop
- run: PRONTO_PULL_REQUEST_ID="$(jq --raw-output .number "$GITHUB_EVENT_PATH")" PRONTO_GITHUB_ACCESS_TOKEN="${{ secrets.GITHUB_TOKEN }}" pronto run -f github_status github_pr -c origin/master
- run: PRONTO_PULL_REQUEST_ID="$(jq --raw-output .number "$GITHUB_EVENT_PATH")" PRONTO_GITHUB_ACCESS_TOKEN="${{ github.token }}" bundle exec pronto run -f github_status github_pr -c origin/${{ github.base_ref }}
Copy link

Choose a reason for hiding this comment

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

I also has to add .pull_request.number vs .number not sure if you experienced this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems working fine for me :-
image

Copy link

Choose a reason for hiding this comment

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

When adding bundle exec the following error occurs Bundler::GemNotFound. I don't believe the Workflow creates a Gemfile, causing this error. Perhaps this should be removed?

Copy link
Contributor Author

@reckerswartz reckerswartz May 13, 2020

Choose a reason for hiding this comment

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

yes, you are right but since I have used with Gemfile and bundler is now part of the lastest ruby.
Running command directly was causing Pronto not found issue. I am thinking to write a wiki for specific integrations where such things can be mentioned.

Copy link

Choose a reason for hiding this comment

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

Yes, but for this README, to @jaydorsey 's comment below, most people will copy and paste this snippet. In order for bundle exec to work, there should be the steps of bundle install etc before hand. If not, simply removing bundle exec allows for the minimum requirements needed to get this working as an isolated event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smridge updated, please review.

Copy link

Choose a reason for hiding this comment

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

Nice Wiki! 👍

README.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@reckerswartz reckerswartz left a comment

Choose a reason for hiding this comment

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

.pull_request.number vs .number (can't determine cause of this)

@reckerswartz reckerswartz removed the request for review from a team May 12, 2020 09:50
README.md Outdated
- name: Checkout code
uses: actions/checkout@v2
- run: |
git fetch --prune --unshallow
Copy link

@jaydorsey jaydorsey May 12, 2020

Choose a reason for hiding this comment

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

What's the benefit of passing in --unshallow? Asking because I usually set my linting pipelines up with a --depth=N (maybe 10 usually?) to reduce the size of the transfer and make the clone faster, because you're only linting a specific commit/snapshot in time

Edit: I know this is just a README example, but was curious if there was a benefit of setting the example, which people are likely to copy/paste from, as this that I'm missing

Copy link
Contributor Author

@reckerswartz reckerswartz May 13, 2020

Choose a reason for hiding this comment

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

@jaydorsey --depth=1 didn't work for me and found --unshallow resolve the issue without much very about depth size. I will create a wiki page and try to specify it over there.
Thanks for your input.

Copy link
Contributor Author

@reckerswartz reckerswartz May 13, 2020

Choose a reason for hiding this comment

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

@jaydorsey updating with

- run: |
          git fetch --no-tags --prune --depth=10 origin +refs/heads/*:refs/remotes/origin/*

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to tag me in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean to tag me in here?

Oops! typo 🤯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jaydorsey updated, please review.

@reckerswartz reckerswartz requested a review from mknapik May 13, 2020 18:09
@reckerswartz
Copy link
Contributor Author

@mknapik can you merge ?

@mknapik mknapik merged commit 52c6a77 into prontolabs:master May 14, 2020
@mknapik
Copy link
Contributor

mknapik commented May 14, 2020

Sure, thanks for your contribution!

@reckerswartz reckerswartz deleted the patch-1 branch May 14, 2020 11:07
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.

5 participants