unity-scope-click

Merge lp:~dobey/unity-scope-click/full-langs into lp:unity-scope-click/devel

  1. full-langs
  2. Merge into devel
Proposed by dobey
Status: Merged
Approved by: dobey
Approved revision: 269
Merged at revision: 269
Proposed branch: lp:~dobey/unity-scope-click/full-langs
Merge into: lp:unity-scope-click/devel
Diff against target: 140 lines (+66/-4)
5 files modified
libclickscope/click/configuration.cpp (+15/-0)
libclickscope/click/configuration.h (+2/-0)
libclickscope/tests/test_configuration.cpp (+12/-0)
scope/click/reviews.cpp (+9/-2)
scope/tests/test_reviews.cpp (+28/-2)
To merge this branch: bzr merge lp:~dobey/unity-scope-click/full-langs
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alejandro J. Cura (community) Approve
Paweł Stołowski (community) Needs Fixing
Review via email: mp+220523@code.launchpad.net

Commit message

Handle some languages differently when submitting reviews.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote :

Looks good, just one minor nitpick:

26 + if (std::find(FULL_LANG_CODES.begin(), FULL_LANG_CODES.end(), language)
27 + != FULL_LANG_CODES.end()) {
28 + return true;
29 + }
30 + return false;

can you reduce it to just:
return std::find(FULL_LANG_CODES.begin(), FULL_LANG_CODES.end(), language)!= FULL_LANG_CODES.end();

review: Needs Fixing
lp:~dobey/unity-scope-click/full-langs updated
269. By dobey

Clean is_full_lang_code function per review.

Revision history for this message
Alejandro J. Cura (alecu) wrote :

Branch looks good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libclickscope/click/configuration.cpp'
2--- libclickscope/click/configuration.cpp 2014-05-16 21:44:07 +0000
3+++ libclickscope/click/configuration.cpp 2014-05-22 16:34:27 +0000
4@@ -40,6 +40,15 @@
5
6 namespace click {
7
8+/* NOTE: The list of languages we need to use the full language code for.
9+ * Please keep this list in a-z order.
10+ */
11+const std::vector<const char*> Configuration::FULL_LANG_CODES = {
12+ "pt_BR",
13+ "zh_CN",
14+ "zh_TW",
15+};
16+
17 std::vector<std::string> Configuration::list_folder(const std::string& folder, const std::string& pattern)
18 {
19 std::vector<std::string> result;
20@@ -121,4 +130,10 @@
21 return result;
22 }
23
24+bool Configuration::is_full_lang_code(const std::string& language)
25+{
26+ return std::find(FULL_LANG_CODES.begin(), FULL_LANG_CODES.end(), language)
27+ != FULL_LANG_CODES.end();
28+}
29+
30 } // namespace click
31
32=== modified file 'libclickscope/click/configuration.h'
33--- libclickscope/click/configuration.h 2014-05-16 21:44:07 +0000
34+++ libclickscope/click/configuration.h 2014-05-22 16:34:27 +0000
35@@ -43,6 +43,7 @@
36 constexpr static const char* FRAMEWORKS_PATTERN {"*.framework"};
37 constexpr static const int FRAMEWORKS_EXTENSION_LENGTH = 10; // strlen(".framework")
38 constexpr static const char* LANGUAGE_ENVVAR {"LANGUAGE"};
39+ static const std::vector<const char*> FULL_LANG_CODES;
40
41 virtual std::vector<std::string> get_available_frameworks();
42 virtual std::string get_architecture();
43@@ -50,6 +51,7 @@
44 virtual std::string get_language_base();
45 virtual std::string get_language();
46 virtual std::string get_accept_languages();
47+ static bool is_full_lang_code(const std::string& language);
48
49 virtual ~Configuration() {}
50 protected:
51
52=== modified file 'libclickscope/tests/test_configuration.cpp'
53--- libclickscope/tests/test_configuration.cpp 2014-05-16 21:44:07 +0000
54+++ libclickscope/tests/test_configuration.cpp 2014-05-22 16:34:27 +0000
55@@ -172,3 +172,15 @@
56 EXPECT_EQ(Configuration().get_accept_languages(), "en");
57 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);
58 }
59+
60+TEST(Configuration, isFullLangCodeTestAllFullLangCodes)
61+{
62+ for (auto lang: Configuration::FULL_LANG_CODES) {
63+ EXPECT_TRUE(Configuration::is_full_lang_code(lang));
64+ }
65+}
66+
67+TEST(Configuration, isFullLangCodeReturnsFalseForEN)
68+{
69+ ASSERT_FALSE(Configuration::is_full_lang_code("en_US"));
70+}
71
72=== modified file 'scope/click/reviews.cpp'
73--- scope/click/reviews.cpp 2014-05-07 21:22:59 +0000
74+++ scope/click/reviews.cpp 2014-05-22 16:34:27 +0000
75@@ -139,8 +139,15 @@
76 root["review_text"] = review.review_text;
77
78 root["arch_tag"] = click::Configuration().get_architecture();
79- // NOTE: We only use the base language code for reviews.
80- root["language"] = click::Configuration().get_language_base();
81+ /* NOTE: We only use the base language code for reviews, except for
82+ * codes in the click::Configuration::FULL_LANG_CODES vector.
83+ */
84+ auto language = click::Configuration().get_language();
85+ if (click::Configuration::is_full_lang_code(language)) {
86+ root["language"] = language;
87+ } else {
88+ root["language"] = click::Configuration().get_language_base();
89+ }
90
91 // NOTE: "summary" is a required field, but we don't have one. Use "".
92 root["summary"] = "Review";
93
94=== modified file 'scope/tests/test_reviews.cpp'
95--- scope/tests/test_reviews.cpp 2014-05-20 19:33:41 +0000
96+++ scope/tests/test_reviews.cpp 2014-05-22 16:34:27 +0000
97@@ -275,8 +275,8 @@
98 review.package_version = "0.1";
99
100 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,
101- "zh_TW.UTF-8", 1), 0);
102- std::string expected_language = "\"language\":\"zh\"";
103+ "es_AR.UTF-8", 1), 0);
104+ std::string expected_language = "\"language\":\"es\"";
105 EXPECT_CALL(*clientPtr, callImpl(_, "POST", true, _,
106 HasSubstr(expected_language), _))
107 .Times(1)
108@@ -287,6 +287,32 @@
109 ASSERT_EQ(unsetenv(click::Configuration::LANGUAGE_ENVVAR), 0);
110 }
111
112+TEST_F(ReviewsTest, testSubmitReviewLanguageCorrectForFullLangCodes)
113+{
114+ LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
115+ auto response = responseForReply(reply.asSharedPtr());
116+
117+ click::Review review;
118+ review.rating = 3;
119+ review.review_text = "A review.";
120+ review.package_name = "com.example.test";
121+ review.package_version = "0.1";
122+
123+ for (std::string lang: click::Configuration::FULL_LANG_CODES) {
124+ ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,
125+ lang.c_str(), 1), 0);
126+ std::string expected_language = "\"language\":\"" + lang + "\"";
127+ EXPECT_CALL(*clientPtr, callImpl(_, "POST", true, _,
128+ HasSubstr(expected_language), _))
129+ .Times(1)
130+ .WillOnce(Return(response));
131+
132+ auto submit_op = reviewsPtr->submit_review(review,
133+ [](click::Reviews::Error){});
134+ ASSERT_EQ(unsetenv(click::Configuration::LANGUAGE_ENVVAR), 0);
135+ }
136+}
137+
138 TEST_F(ReviewsTest, testGetBaseUrl)
139 {
140 const char *value = getenv(click::REVIEWS_BASE_URL_ENVVAR.c_str());

Subscribers

People subscribed via source and target branches

to all changes: