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

Data Collection Practices #1191

Merged
merged 4 commits into from Nov 20, 2020
Merged

Data Collection Practices #1191

merged 4 commits into from Nov 20, 2020

Conversation

bpoplauschi
Copy link
Member

New Pull Request Checklist

This merge request fixes / refers to the following issues: #1187

Pull Request Description

Add README section for the Data Collection Practices

@bpoplauschi
Copy link
Member Author

@CocoaLumberjack/collaborators let's see some opinions here on how we can add more value to our framework's users with this section of the docs.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Nov 16, 2020

Codecov Report

Merging #1191 (823acdc) into master (48c2b96) will decrease coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1191      +/-   ##
==========================================
- Coverage   46.40%   46.23%   -0.18%     
==========================================
  Files          29       29              
  Lines        4663     4663              
==========================================
- Hits         2164     2156       -8     
- Misses       2499     2507       +8     
Impacted Files Coverage Δ
Sources/CocoaLumberjack/DDOSLogger.m 65.21% <0.00%> (-17.40%) ⬇️

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 48c2b96...823acdc. Read the comment docs.

Converted markdown headings from h3 to h2 and from h4 to h3 to better highlight section headers (we were not using h2 until now)
Copy link
Member

@sushichop sushichop left a comment

Choose a reason for hiding this comment

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

Great work! IMO, this is enough👍

@bpoplauschi
Copy link
Member Author

Cool @sushichop, thanks for looking into this.
I would like to also get @ffried's opinion before merging, just to be on the safe side :)

@sushichop
Copy link
Member

@bpoplauschi

I would like to also get @ffried's opinion before merging, just to be on the safe side :)

I feel the same way🙂

@bpoplauschi
Copy link
Member Author

@sushichop I just wanted both your opinions before merging ;)

@sushichop
Copy link
Member

@bpoplauschi

@sushichop I just wanted both your opinions before merging ;)

Yeah, Thanks. Let's wait for a response🙂

Copy link
Member

@ffried ffried left a comment

Choose a reason for hiding this comment

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

Looks good in general. 👍
Just found two superfluous whitespaces and suggested a small re-phrasing of one sentence.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lolgear
Copy link
Contributor

lolgear commented Nov 19, 2020

@bpoplauschi
Am I right that you said about "docs" in related issue, but this PR only affects Readme.
Not sure if Readme is a document that is convenient for app developers.
Should we add additional document about data collection?

@bpoplauschi
Copy link
Member Author

@ffried thanks for taking a look, I agree with your suggestions and have fixed those in a commit.
@lolgear once this PR is merged, I will regenerate the doc on https://cocoalumberjack.github.io which includes the content from our Readme, so that will also live on our documentation main page. I think this is enough.

@bpoplauschi bpoplauschi merged commit f7ee274 into master Nov 20, 2020
@bpoplauschi bpoplauschi deleted the data_collection_practices branch November 20, 2020 15:44
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.

None yet

4 participants