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

Remove deprecations #1730

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Remove deprecations #1730

merged 1 commit into from
Feb 4, 2021

Conversation

sequencer
Copy link
Member

@sequencer sequencer commented Jan 14, 2021

Depends on freechipsproject/chisel-testers#309

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you state the API impact?
  • Did you specify the code generation impact?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • code cleanup

API Impact

Remove all deprecated API in before and include 3.2

Desired Merge Strategy

  • Rebase: You will rebase the PR onto master and it will be merged with a merge commit.

Release Notes

Remove deprecations since and include 3.2

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (3.2.x, 3.3.x, 3.4.0, 3.5.0) ?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you mark as Please Merge?

* then continues trimming elements belonging to [[blacklistPackages]]
*/
@deprecated("This method will be removed in 3.4", "3.3")
def trimmedStackTrace: Array[StackTraceElement] = trimmedStackTrace(this)
Copy link
Member Author

Choose a reason for hiding this comment

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

Although it's deprecated in 3.3, I removed this since it's a internal API and no other place use it.
And it claims to be removed in in 3.4. ;p

@sequencer sequencer requested review from jackkoenig and seldridge and removed request for jackkoenig January 14, 2021 09:45
@sequencer sequencer changed the title remove deprecations since 3.2 Remove deprecations and Scala 2.11 Jan 14, 2021
@sequencer sequencer force-pushed the remove_deprecations branch from 51569e5 to ef0e5b0 Compare January 14, 2021 11:26
@sequencer sequencer mentioned this pull request Jan 14, 2021
14 tasks
@jackkoenig
Copy link
Contributor

I like the idea here, but we unfortunately can't remove everything that's deprecated. The chisel-iotesters only use chisel3.Driver and ExecutionOptionsManager so until either they stop or we stop supporting chisel-iotesters, we're kind of stuck with those. @chick @seldridge is there anything else we're stuck with at the moment?

If you could put remove 2.11 in a different PR I'd love to fast track that one 🚀

@jackkoenig
Copy link
Contributor

I think we've discussed pulling ExecutionOptionsManager out of chisel3 and firrtl and into chisel-iotesters, if someone wants to tackle that that would vastly clean up the code here.

@sequencer
Copy link
Member Author

If you could put remove 2.11 in a different PR I'd love to fast track that one rocket

This PR is rebase onto #1732 now ;)

The chisel-iotesters only use chisel3.Driver and ExecutionOptionsManager so until either they stop or we stop supporting chisel-iotesters, we're kind of stuck with those.

I will take a look today.

@sequencer sequencer force-pushed the remove_deprecations branch from ef0e5b0 to 9c12c12 Compare January 15, 2021 05:24
@sequencer sequencer changed the title Remove deprecations and Scala 2.11 Remove deprecations Jan 15, 2021
@sequencer
Copy link
Member Author

The chisel-iotesters only use chisel3.Driver and ExecutionOptionsManager so until either they stop or we stop supporting chisel-iotesters, we're kind of stuck with those. @chick @seldridge is there anything else we're stuck with at the moment?

Just take another hour on freechipsproject/chisel-testers#309, it should not be a blocker now:

[info] ScalaTest
[info] Run completed in 3 minutes, 43 seconds.
[info] Total number of tests run: 109
[info] Suites: completed 49, aborted 0
[info] Tests: succeeded 109, failed 0, canceled 2, ignored 1, pending 0
[info] All tests passed.
[info] Passed: Total 109, Failed 0, Errors 0, Passed 109, Ignored 1, Canceled 2
[success] Total time: 258 s (04:18), completed Jan 15, 2021, 5:56:20 PM

@sequencer sequencer force-pushed the remove_deprecations branch 2 times, most recently from 6959cd0 to 5c1e642 Compare January 21, 2021 01:07
@sequencer sequencer force-pushed the remove_deprecations branch from 5c1e642 to 8391247 Compare January 26, 2021 19:12
@chick
Copy link
Contributor

chick commented Jan 27, 2021

@sequencer

repo branch hash
firrtl master aec9e9e
firrtl-interpeter master 937c44d7ca19c3daca8eea9c52cbaff5476c311b
treadle master 0ec32a5112a2cf0b9741790cf8c8ebb4b8794ddf
chisel3 remove_deprecations 8391247

chisel3 built and passed tests.

@jackkoenig
Copy link
Contributor

@chick did you check upstream repos? chisel-iotesters (with the related PR), chisel-test, and dsp-tools?

@chick
Copy link
Contributor

chick commented Jan 29, 2021

I started testing but got stuck at iotesters. I talked to @sequencer and he said he would fix.

@sequencer
Copy link
Member Author

I tested this, those was introduced by remove io. I’ll fix that today.

Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

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

:shipit:

@jackkoenig jackkoenig merged commit f45216e into master Feb 4, 2021
@jackkoenig jackkoenig deleted the remove_deprecations branch February 4, 2021 00:36
sequencer added a commit to sequencer/barstools that referenced this pull request Apr 3, 2021
sequencer added a commit to sequencer/barstools that referenced this pull request Apr 3, 2021
sequencer added a commit to sequencer/firesim that referenced this pull request Apr 6, 2021
This commit remove all reference to chisel3.core, toBool, unless in firesim,
which was broken by chipsalliance/chisel#1730.
sequencer added a commit to sequencer/firesim that referenced this pull request Apr 6, 2021
This commit remove all reference to chisel3.core, toBool, unless in firesim,
which was broken by chipsalliance/chisel#1730.
chick added a commit to ucb-bar/dsptools that referenced this pull request Apr 19, 2021
This compbined PR #222 AND PR #222

Depends on freechipsproject/chisel-testers#309
Depends on chipsalliance/chisel#1730

Black boxes required suggest Name to work
setResource on black box is not addResource
Lots of import cleanup
Saturate did it's own firrtl compiling, modified to use current idioms
Changed dsptools Driver to use jiuyangs iotesters DriverCompatibility stuff
Cleaned up dependencies in build.sbt
Removed BuildInfo plugin from build.sbt (I'm not that sure about it, but it was causing build errors)
--
Now uses github actions for CI
chick added a commit to ucb-bar/dsptools that referenced this pull request Apr 19, 2021
* Black boxes required suggest Name to work
setResource on black box is not addResource
Lots of import cleanup
Saturate did it's own firrtl compiling, modified to use current idioms
Changed dsptools `Driver` to use jiuyangs iotesters DriverCompatibility stuff
Cleaned up dependencies in build.sbt
Removed BuildInfo plugin from build.sbt (I'm not that sure about it, but it was causing build errors)

* Add support for github actions to do ci and publishing

* Add support for github actions to do ci and publishing
Added CODEOWNERS file for symmetry with chiseltest which was used for this template
Removed mill as it is not currently supported here

* Update plugins to have ci-release, used for publishing

* fix iotesters version

* bump copyright to force retest

* Move to github actions for ci, update for 3.5 compatabilty
This compbined PR #222 AND PR #222

Depends on freechipsproject/chisel-testers#309
Depends on chipsalliance/chisel#1730

Black boxes required suggest Name to work
setResource on black box is not addResource
Lots of import cleanup
Saturate did it's own firrtl compiling, modified to use current idioms
Changed dsptools Driver to use jiuyangs iotesters DriverCompatibility stuff
Cleaned up dependencies in build.sbt
Removed BuildInfo plugin from build.sbt (I'm not that sure about it, but it was causing build errors)
--
Now uses github actions for CI

* Only build for scala 2.12
@sequencer sequencer mentioned this pull request May 15, 2021
14 tasks
sequencer added a commit to sequencer/firesim that referenced this pull request Jun 7, 2021
This commit remove all reference to chisel3.core, toBool, unless in firesim,
which was broken by chipsalliance/chisel#1730.
sequencer added a commit to sequencer/firesim that referenced this pull request Jun 7, 2021
This commit remove all reference to chisel3.core, toBool, unless in firesim,
which was broken by chipsalliance/chisel#1730.
sequencer added a commit to sequencer/barstools that referenced this pull request Jul 12, 2021
sequencer added a commit to sequencer/barstools that referenced this pull request Jul 13, 2021
jackkoenig pushed a commit that referenced this pull request Feb 28, 2023
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Jiuyang Liu <[email protected]>
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