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

chore(unsplash): deprecate unused const #1046

Merged
merged 11 commits into from
Jun 17, 2022
Merged

chore(unsplash): deprecate unused const #1046

merged 11 commits into from
Jun 17, 2022

Conversation

import-brain
Copy link
Member

This is a todo on "source code todo list v2"

@import-brain import-brain added c: chore PR that doesn't affect the runtime behavior p: 1-normal Nothing urgent labels Jun 9, 2022
@import-brain import-brain added this to the v7 - Current Major milestone Jun 9, 2022
@import-brain import-brain requested a review from a team June 9, 2022 14:38
@import-brain import-brain requested a review from a team as a code owner June 9, 2022 14:38
@import-brain import-brain self-assigned this Jun 9, 2022
@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #1046 (e660c69) into main (f4d23eb) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1046      +/-   ##
==========================================
- Coverage   99.64%   99.64%   -0.01%     
==========================================
  Files        2135     2135              
  Lines      229213   229217       +4     
  Branches      980      979       -1     
==========================================
+ Hits       228402   228405       +3     
- Misses        790      791       +1     
  Partials       21       21              
Impacted Files Coverage Δ
src/modules/image/providers/unsplash.ts 95.32% <100.00%> (+0.11%) ⬆️
src/modules/finance/index.ts 99.31% <0.00%> (-0.69%) ⬇️
src/modules/internet/user-agent.ts 86.95% <0.00%> (+0.57%) ⬆️

@import-brain import-brain requested review from xDivisionByZerox, Shinigami92 and ST-DDT and removed request for a team June 11, 2022 23:15
Co-authored-by: xDivisionByZerox <[email protected]>
@import-brain
Copy link
Member Author

@xDivisionByZerox Would you prefer I write a test case for the deprecation log, or is it fine in this case?

@xDivisionByZerox
Copy link
Member

Normally our procedure when deprecating something consists of 3 steps:

  1. Use the internal deprecated function
  2. Add a @deprecated JSDoc parameter
  3. Add a test case that ensures that the deprecation log is being printed to the console

So I guess to be exact a test would be required, if you don't mind.

@import-brain
Copy link
Member Author

Normally our procedure when deprecating something consists of 3 steps:

  1. Use the internal deprecated function
  2. Add a @deprecated JSDoc parameter
  3. Add a test case that ensures that the deprecation log is being printed to the console

So I guess to be exact a test would be required, if you don't mind.

Sure, I'll get on it :)

Co-authored-by: xDivisionByZerox <[email protected]>
ST-DDT
ST-DDT previously approved these changes Jun 12, 2022
@ST-DDT ST-DDT requested review from a team June 12, 2022 15:36
ST-DDT
ST-DDT previously approved these changes Jun 13, 2022
@ST-DDT ST-DDT requested a review from a team June 13, 2022 19:37
@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Jun 13, 2022
test/image.spec.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT requested a review from a team June 14, 2022 19:07
@xDivisionByZerox xDivisionByZerox changed the title chore: remove unused const in unsplash.ts chore: deprecate unused const in unsplash.ts Jun 16, 2022
@xDivisionByZerox xDivisionByZerox changed the title chore: deprecate unused const in unsplash.ts chore(unsplash): deprecate unused const Jun 16, 2022
@ST-DDT ST-DDT enabled auto-merge (squash) June 17, 2022 07:12
@ST-DDT ST-DDT merged commit 029b062 into main Jun 17, 2022
@ST-DDT ST-DDT deleted the remove_unused_const branch June 18, 2022 17:02
Minozzzi pushed a commit to Minozzzi/faker that referenced this pull request Jul 19, 2022
@xDivisionByZerox xDivisionByZerox added the m: image Something is referring to the image module label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: chore PR that doesn't affect the runtime behavior m: image Something is referring to the image module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants