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

feat: split rustywind into multiple crates so that it can also be made available as a library #100

Merged
merged 8 commits into from
Apr 12, 2024

Conversation

Rolv-Apneseth
Copy link
Contributor

@Rolv-Apneseth Rolv-Apneseth commented Apr 5, 2024

In relation to #95

Continuing on from bram209's branch, so most of the credit goes to him.

Opening this PR now because I need some pointers:

  1. That latest commit I pushed, was just replacing the default WriteMode::DryRun with WriteMode::ToConsole because clippy is complaining that ToConsole is never constructed, and DryRun already has an option dedicated to it. If this is wrong, would you mind clarifying what the purpose of ToConsole is, and how I should fix that linting error?

  2. For adding a workflow to publish rustywind_core, would that just be something like this (found on the internet):

    on:
      push:
        tags:        
          - '*'
      workflow_dispatch:
    
    name: Publish Library
    
    jobs:
      publish:
        name: Publish
        runs-on: ubuntu-latest
        steps:
          - name: Checkout sources
            uses: actions/checkout@v2
    
          - name: Install stable toolchain
            uses: actions-rs/toolchain@v1
            with:
              profile: minimal
              toolchain: stable
              override: true
    
          - run: cargo publish --package rustywind_core --token ${CRATES_TOKEN}
            env:
              CRATES_TOKEN: ${{ secrets.CRATES_TOKEN }}

    or should it be using an action like this?

  3. For rustywind_vite, what should the description be? And is this also to be published as a separate library?

  4. Should versioning be different for all 3 crates?

  5. How can I check CI/CD workflows are working? I have ensured clippy and rustfmt are happy locally.

Did I miss anything else?

@praveenperera
Copy link
Member

Hey @Rolv-Apneseth sorry I didn’t see this till now. I won’t be able to review till Tuesday.

  1. That’s good console as default makes sense
  2. I don’t need this for now I’ll do releases manually
  3. Yes let’s just publish as another package, the point it to make rustywind option to automatically get the class order from the output css file to a work with vite.
  4. Yes
  5. Running now.

Is this ready to review now? Or does it need to be in draft longer?

@Rolv-Apneseth
Copy link
Contributor Author

Sure, no rush at all.

Sorry, wanted to ask those questions first, it can be ready for review yeah. Just need to change the versions and look into that workflow failing

@Rolv-Apneseth Rolv-Apneseth marked this pull request as ready for review April 7, 2024 22:56
@Rolv-Apneseth
Copy link
Contributor Author

Not sure what the issue was with that workflow failing, globbing was broken somehow? I could maybe change it to just run cargo fmt --check -- --edition 2021

@praveenperera
Copy link
Member

Not sure what the issue was with that workflow failing, globbing was broken somehow? I could maybe change it to just run cargo fmt --check -- --edition 2021

Ya let’s change it to that

@Rolv-Apneseth
Copy link
Contributor Author

Rolv-Apneseth commented Apr 7, 2024

Cool I'll change it. Is this comment still relevant?

# When pushed to master, run `cargo +nightly fmt --all` and open a PR.

Edit:

Also actions-rs/toolchain has been archived. Could I try change it to something like this, like I have here:

jobs:
    rustfmt:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v4

            - name: Install stable toolchain
              run: rustup toolchain install stable

            - name: Format
              run: cargo fmt --check -- --edition 2021

@Rolv-Apneseth
Copy link
Contributor Author

Oh, or this one from dtolnay if you prefer

@praveenperera
Copy link
Member

No that can be removed

@Rolv-Apneseth
Copy link
Contributor Author

No that can be removed

Alright done. Any thoughts about changing the action or leave it as is?

@praveenperera
Copy link
Member

looks good @Rolv-Apneseth thanks! I'll merge it in today.

@praveenperera praveenperera merged commit 67b2318 into avencera:master Apr 12, 2024
12 checks passed
@Rolv-Apneseth
Copy link
Contributor Author

Nice, thank you

@praveenperera
Copy link
Member

Added some docs: https://docs.rs/rustywind_core/latest/rustywind_core/

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.

3 participants