Skip to content

Conversation

@Ostrenkiy
Copy link
Contributor

Задача: #APPS-2078

Описание:
Немного переписал то, как работает модуль для сплит-тестов. Теперь группа отсылаетя как user property при запуске приложения, а для каждого теста можно определять, должен ли пользователь в a/b-тесте участвовать.
Суть теста описана в задаче.

@Ostrenkiy Ostrenkiy added this to the 1.70 milestone Oct 16, 2018
@Ostrenkiy Ostrenkiy self-assigned this Oct 16, 2018
@Ostrenkiy Ostrenkiy requested review from ivan-magda and kvld October 16, 2018 19:22
@Ostrenkiy Ostrenkiy changed the base branch from master to dev October 16, 2018 19:55
self.analytics = analytics
}

typealias GroupType = Group
Copy link
Contributor

Choose a reason for hiding this comment

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

Давай наверх вынесем

}
}

extension AuthAfterOnboardingSplitTest.Group {
Copy link
Contributor

Choose a reason for hiding this comment

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

Кажется, что экстеншен не нужен. Енум же AuthAfterOnboardingSplitTest.Group, можно сразу всё описать

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Мне кстати кажется, что здесь с экстеншном удобнее. Давай просто договоримся, что там будем пилить все, что касается поведения групп (все, что не в этом экстеншне, можно +- копипастить из теста в тест, смысловой нагрузки там мало)

Copy link
Contributor

Choose a reason for hiding this comment

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

Сделаем так, как решит третий @vanyaland

Copy link
Member

Choose a reason for hiding this comment

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

В данном случае расширение ради одной переменной мне кажется излишне и можно сразу описать Group

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙄

if let startVersion = defaults.value(forKey: startVersionKey) as? String {
return startVersion
} else {
let version = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as! String
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно попробовать безопаснее

@Ostrenkiy
Copy link
Contributor Author

Пофиксил

@Ostrenkiy Ostrenkiy merged commit b7760f9 into dev Oct 17, 2018
@Ostrenkiy Ostrenkiy deleted the feature/auth-after-onboarding-ab branch October 17, 2018 11:48
@kvld kvld mentioned this pull request Oct 18, 2018
@ivan-magda ivan-magda added the a/b Pull requests that working with A/B testing label Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a/b Pull requests that working with A/B testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants