-
Notifications
You must be signed in to change notification settings - Fork 640
Support mdoc from Mill and remove SBT #4727
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
Conversation
| // See LICENSE for license details. | ||
|
|
||
| package build.docs | ||
|
|
||
| import github4s.Github | ||
| import github4s.Github._ | ||
| import github4s.domain.User |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping git would detect these new .mill files (other than docs/package.mill) as moves with modification from the old .scala files in project/ but it doesn't seem to.
Regardless, these are moves, feel free to review their code but just note they are very much existing code (again, other than docs/package.mill).
|
Ask @unlsycn to take a look at this since you have been helping us on the build system infrastructure, sorry I have been busy with our new uarch designing. Please test if this bumping works in our platform and see if there are anything can be polished? |
| run: ./mill -j0 firrtl.cross[].test -oF + svsim.cross[].test -oF + chisel[].test -oF | ||
| - name: Binary compatibility | ||
| # TODO either make this also check the plugin or decide that we don't | ||
| # support binary compatibility for the plugin | ||
| run: sbt ++${{ inputs.scala }} unipublish/mimaReportBinaryIssues | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a replacement for this yet, there is mill-mdoc which looks like it was upgraded to mill 0.12 but hasn't been published yet--unclear but will try it out later. In nay case, this doesn't actually do anything on main as we don't have prior artifacts yet, so I think it's okay to drop temporarily.
This includes porting extra functionality from project/
84e66fe to
f91bf9e
Compare
LGTM and this PR works fine with t1 |
|
Thanks for the review @unlsycn! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of scope for this PR: we should delete this and move to a model where the contributors are handled with a CI flow like https://github.com/chipsalliance/firrtl-spec/blob/main/.github/workflows/update-contributors.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that is better, will get to it eventually 🙃
This includes porting extra functionality from
project/Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.