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

Feature/open source template #42

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ZvonimirMedak
Copy link
Contributor

Summary

I have updated the repository to be closer in line with the new open-source-template

Changes

  • Readme.md
  • Contributing.md
  • Add Codeowners file
  • remove bug, and feature request md files
  • add bug, and feature request yml files
  • update the pull_request_template.md

Type

  • Feature: This pull request introduces a new feature.
  • Bug fix: This pull request fixes a bug.
  • Refactor: This pull request refactors existing code.
  • Documentation: This pull request updates documentation.
  • Other: This pull request makes other changes.

Additional information

  • [] This pull request introduces a breaking change.

Description

  • As codeowners I have added Nikola, and me
  • I have updated the readme with a bit of word styling, added some new parts to it, and some code snippets
  • other files were updated based on their new look in the open source template

Checklist

  • I have performed a self-review of my own code.
  • I have tested my changes, including edge cases.
  • I have added necessary tests for the changes introduced (if applicable).
  • I have updated the documentation to reflect my changes (if applicable).

Additional notes

The rest of the updates will be done through other pull requests

@ZvonimirMedak ZvonimirMedak added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 14, 2024
@ZvonimirMedak ZvonimirMedak self-assigned this Nov 14, 2024
Copy link
Member

@nikolamajcen nikolamajcen left a comment

Choose a reason for hiding this comment

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

I've added some comments, but overall, looks good 👍

@@ -28,7 +30,7 @@ This library supports both **Swift** and **Objective-C**.

## Requirements

* iOS 10 and above
* iOS 11 and above
* Xcode 10 and above
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this would work on Xcode 10.

UserDefaultsTool(),
CustomLocationTool()
],
preferences: optionSwitchItems
Copy link
Member

Choose a reason for hiding this comment

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

Please add optionSwitchItems somewhere because it is hard to understand what they are in this example.

e.g. The app supports Analitycs and you can add an `OptionSwitchTool` which will be shown on the `Preferences` screen and the user can turn it off if he doesn't want it.

```swift
OptionSwitchItem(
Copy link
Member

Choose a reason for hiding this comment

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

Remove indentation for this example.

@@ -0,0 +1 @@
* @nikolamajcen @ZvonimirMedak
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I'm totally fine that you set only yourself here as a maintainer. I could be added manually any time you need me 👍

)

Sentinel.shared.setup(with: configuration)
let configuration = Sentinel.Configuration(
Copy link
Member

Choose a reason for hiding this comment

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

Remove indentation.

Comment on lines 7 to 10
[![Build Status](https://app.bitrise.io/app/56dc4082e9c3bb9e/status.svg?token=aHG6rIR2XDrJ3xNOIO2hXw&branch=master)](https://app.bitrise.io/app/56dc4082e9c3bb9e)
[![Version](https://img.shields.io/cocoapods/v/Sentinel.svg?style=flat)](https://cocoapods.org/pods/Sentinel)
[![License](https://img.shields.io/cocoapods/l/Sentinel.svg?style=flat)](https://cocoapods.org/pods/Sentinel)
[![License](https://img.shields.io/cocoapods/l/Sentinel.svg?style=flat)](https://cocoapods.org/pods/Sentinel#license)
[![Platform](https://img.shields.io/cocoapods/p/Sentinel.svg?style=flat)](https://cocoapods.org/pods/Sentinel)
Copy link
Member

Choose a reason for hiding this comment

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

For the better appearance, you can center this if you want :)

#### Swift Package Manager

If you are using SPM for your dependency manager, add this to the dependencies in your `Package.swift` file:

```swift
dependencies: [
.package(url: "https://github.com/infinum/ios-sentinel.git")
.package(url: "https://github.com/infinum/ios-sentinel.git")
Copy link
Member

Choose a reason for hiding this comment

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

Indentation needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants