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.

Sign up for GitHub

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

Jump to bottom

Add configure orientation for devcard #406

Merged
merged 3 commits into from
Feb 20, 2024
Merged

Add configure orientation for devcard #406

merged 3 commits into from
Feb 20, 2024

Conversation

ncdai
Copy link
Contributor

@ncdai ncdai commented Feb 20, 2024

Add configure orientation for devcard

  • default: Vertical (Default)
  • wide: Horizontal

Example usage:

jobs:
  devcard:
    runs-on: ubuntu-latest
    permissions:
      contents: write
    steps:
      - name: devcard
        uses: dailydotdev/action-devcard@3.0.0
        with:
          user_id: ${{ secrets.USER_ID }}
          type: wide

Hope my contribution will be accepted. Thank you for the great Action!

@CLAassistant
Copy link

CLAassistant commented Feb 20, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ncdai ncdai mentioned this pull request Feb 20, 2024
Closed
omBratteng
omBratteng requested changes Feb 20, 2024
View reviewed changes
Copy link
Member

@omBratteng omBratteng left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Please do not include the dist/index.js and the yarn.lock changes.

src/index.ts Outdated Show resolved Hide resolved
@ncdai ncdai closed this Feb 20, 2024
@ncdai ncdai reopened this Feb 20, 2024
kopancek
kopancek reviewed Feb 20, 2024
View reviewed changes
src/index.ts Outdated Show resolved Hide resolved
@ncdai
fix: change 'default' and 'wide' to 'Vertical' and 'Horizontal' respe…
afb4946 
…ctively

feat: use enum for DevCardType to improve type safety and readability
feat: update devcardURL function to use DevCardType enum and default to Vertical
feat: update action.yml description for type input to reflect available options Vertical and Horizontal
@ncdai ncdai requested a review from kopancek February 20, 2024 13:59
kopancek
kopancek approved these changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@kopancek kopancek left a comment

Choose a reason for hiding this comment

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

Nice, looks great! 👍 :shipit:

@ncdai
Copy link
Contributor Author

ncdai commented Feb 20, 2024

Nice, looks great! 👍 :shipit:

@kopancek Sorry, I just discovered the error. Please wait for me to update a bit.

kopancek
kopancek suggested changes Feb 20, 2024
View reviewed changes
Copy link
Contributor

@kopancek kopancek left a comment

Choose a reason for hiding this comment

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

Sorry, tests are failing now, because of the default value. Also might make sense to fix the README.md

README.md Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@ncdai
docs(README.md): update optional configuration for devcard orientatio…
90c9eff 
…n to use

"default" and "wide" values instead of "Vertical" and "Horizontal"

fix(action.yml): update input description for type to reflect the new
"default" and "wide" values for devcard orientation configuration.
@ncdai ncdai requested a review from kopancek February 20, 2024 14:04
omBratteng
omBratteng reviewed Feb 20, 2024
View reviewed changes
Copy link
Member

@omBratteng omBratteng left a comment

Choose a reason for hiding this comment

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

lgtm, will merge once @kopancek approves

omBratteng
omBratteng approved these changes Feb 20, 2024
View reviewed changes
Copy link
Member

@omBratteng omBratteng left a comment

Choose a reason for hiding this comment

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

lgtm, will merge once @kopancek approves

@kopancek kopancek merged commit ab4b5bf into dailydotdev:main Feb 20, 2024
3 checks passed
@lcrojano lcrojano mentioned this pull request Feb 27, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omBratteng omBratteng omBratteng approved these changes

@kopancek kopancek kopancek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ncdai @CLAassistant @omBratteng @kopancek