Skip to content

Conversation

@prakashsurya
Copy link
Contributor

No description provided.

@prakashsurya prakashsurya requested review from pzakha and sdimitro May 12, 2020 19:07
@prakashsurya
Copy link
Contributor Author

This is depends on: delphix/actions#3 and delphix/actions#4

- uses: actions/checkout@v2
- run: ./.github/scripts/install-shfmt.sh
- run: /usr/local/bin/shfmt -d .
- uses: delphix/actions/shfmt@master
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't also run shellcheck for sdb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see it used anywhere in this repository, do you? Did I overlook something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant that as a generic question. @sdimitro Is there an explicit reason we aren't running shellcheck? Should we add it?

Copy link
Contributor

Choose a reason for hiding this comment

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

No explicit reason - I just only knew about shfmt at the time when I made that change :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, we can perhaps consider adding it in a later commit.

Copy link
Contributor

@sdimitro sdimitro left a comment

Choose a reason for hiding this comment

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

ShipIt pending test doesn't fail once the other commits make it in.

@prakashsurya prakashsurya force-pushed the use-delphix-actions branch from 502b654 to a84b015 Compare May 13, 2020 17:37
@prakashsurya prakashsurya merged commit 78f6063 into delphix:master May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants