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

refactor(coap-server): simplify form generation #1084

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented Sep 14, 2023

While preparing to implement #1014, I noticed that there is actually some room for simplification in the current CoAP server implementation. This PR makes the corresponding changes, getting rid of two unnecessary helper functions I previously introduced.

@JKRhb JKRhb added the binding-coap Issues related to coap protocol binding label Sep 14, 2023
@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.04% ⚠️

Comparison is base (0cbecdc) 75.35% compared to head (9d22472) 75.31%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1084      +/-   ##
==========================================
- Coverage   75.35%   75.31%   -0.04%     
==========================================
  Files          80       80              
  Lines       16083    16060      -23     
  Branches     1503     1503              
==========================================
- Hits        12119    12096      -23     
  Misses       3925     3925              
  Partials       39       39              
Files Changed Coverage Δ
packages/binding-coap/src/coap-server.ts 78.37% <100.00%> (-0.57%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@relu91 relu91 left a comment

Choose a reason for hiding this comment

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

LGTM, just one question. You removed the port parameter because it was only used to log the Href Assignment and it could have been retrieved using this.port right?

@JKRhb
Copy link
Member Author

JKRhb commented Sep 15, 2023

LGTM, just one question. You removed the port parameter because it was only used to log the Href Assignment and it could have been retrieved using this.port right?

Exactly :)

@JKRhb
Copy link
Member Author

JKRhb commented Sep 15, 2023

Given @relu91's approval, I would merge this one, hope that's fine :)

@JKRhb JKRhb merged commit 227be53 into eclipse-thingweb:master Sep 15, 2023
7 checks passed
@JKRhb JKRhb deleted the coap-form-generation branch September 15, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding-coap Issues related to coap protocol binding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants