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

Deprecate Record.connect #368

Closed
wants to merge 1 commit into from
Closed

Conversation

@awygle
Copy link
Collaborator

@awygle awygle commented Apr 24, 2020

Deprecate the unfortunate Record.connect method, and the Direction functionality of Record as well. Resolves #342 .

@codecov
Copy link

@codecov codecov bot commented Apr 24, 2020

Codecov Report

Merging #368 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   82.56%   82.58%   +0.02%     
==========================================
  Files          35       35              
  Lines        5952     5962      +10     
  Branches     1211     1213       +2     
==========================================
+ Hits         4914     4924      +10     
+ Misses        872      871       -1     
- Partials      166      167       +1     
Impacted Files Coverage Δ
nmigen/hdl/rec.py 97.00% <100.00%> (+0.09%) ⬆️
nmigen/build/run.py 31.25% <0.00%> (ø)
nmigen/hdl/ir.py 95.47% <0.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed0f508...c88a927. Read the comment docs.

@whitequark
Copy link
Member

@whitequark whitequark commented Apr 24, 2020

I'm wondering if we should hold off merging this until #355 is done, since until #355 is done, we really can't encourage people to add their own methods to Record.

@awygle
Copy link
Collaborator Author

@awygle awygle commented Apr 24, 2020

That is true (hence the deprecation message saying to connect fields manually). I am OK with holding off.

@whitequark whitequark marked this pull request as draft Apr 24, 2020
@whitequark
Copy link
Member

@whitequark whitequark commented Jul 8, 2020

Since (as discussed on the last meeting) we're likely going to deprecate Record entirely and instead provide packed structs and interfaces, I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants