Issue24 - Link allowing opening a bug for a typo #53

Merged
merged 4 commits into from Apr 6, 2013

Conversation

Projects
None yet
3 participants
@tx2z
Collaborator

tx2z commented Apr 4, 2013

that's all (I think). Tell me about errors, changes, opinions..etc :)

@tx2z

This comment has been minimized.

Show comment Hide comment
@tx2z

tx2z Apr 4, 2013

Collaborator

I've seem that there are 11 Scrutinizer comments (I don´t know the tool :S) but seem that my last changes are not related to this comments. Maybe we have to open a new issue for that?

Collaborator

tx2z commented Apr 4, 2013

I've seem that there are 11 Scrutinizer comments (I don´t know the tool :S) but seem that my last changes are not related to this comments. Maybe we have to open a new issue for that?

@pascalchevrel

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 4, 2013

Owner

Scrutinizer is a tool for code quality I recently plugged to our repo, we'll see if it is useful (if not, I will disable it) but I don't intend us to follow all advices it gives to the letter :)

I can't look at your patch today, I am working on a Firefox release now, maybe tomorrow

Owner

pascalchevrel commented Apr 4, 2013

Scrutinizer is a tool for code quality I recently plugged to our repo, we'll see if it is useful (if not, I will disable it) but I don't intend us to follow all advices it gives to the letter :)

I can't look at your patch today, I am working on a Firefox release now, maybe tomorrow

@tx2z

This comment has been minimized.

Show comment Hide comment
@tx2z

tx2z Apr 4, 2013

Collaborator

Sure, no problem at all :)

Tell me if you think any change is needed.

About Scrutinizer for me is perfect to know and improve my php skills :D

Collaborator

tx2z commented Apr 4, 2013

Sure, no problem at all :)

Tell me if you think any change is needed.

About Scrutinizer for me is perfect to know and improve my php skills :D

@filip42

This comment has been minimized.

Show comment Hide comment
@filip42

filip42 Apr 4, 2013

Collaborator

Seems ok for me.
Ph

Collaborator

filip42 commented Apr 4, 2013

Seems ok for me.
Ph

web/classes/Transvision/Utils.php
+ if (!file_exists('bugzilla_components.txt') || filemtime('bugzilla_components.txt')+ (7 * 24 * 60 * 60) < time() ) {
+ $json_url = "https://bugzilla.mozilla.org/jsonrpc.cgi?method=Product.get&params=[%20{%20%22names%22:%20[%22Mozilla%20Localizations%22]}%20]";
+ $json = file_get_contents($json_url);
+ file_put_contents('bugzilla_components.txt', file_get_contents($json_url));

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

either use the $json variable or don"t define it

@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

either use the $json variable or don"t define it

web/classes/Transvision/Utils.php
+ */
+ public static function collectLanguageComponent($actual_lng, $components_array)
+ {
+ $component_string = "Other";

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

nit: single quotes

@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

nit: single quotes

web/classes/Transvision/Utils.php
+ {
+ $component_string = "Other";
+ foreach ($components_array as $component) {
+ if (strpos($component['name'],$actual_lng) !== false) {

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

nit: space after coma parameter

@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

nit: space after coma parameter

web/views/results.php
@@ -15,10 +15,15 @@
$results = new ShowResults();
$search_results = $results->getTMXResults(array_keys($locale1_strings), $tmx_source, $tmx_target);
-echo '<h2><span class="searchedTerm">' . $initial_search . '</span> is in ' . $sourceLocale . ' in:</h2>';
+// Get cached bugzilla components (languages list) or connect to Bugzilla API to retreive them

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

nit: retrieve

@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

nit: retrieve

web/classes/Transvision/ShowResults.php
+ $component = rawurlencode($target_component_name);
+ //Bug message
+ $bug_summary = rawurlencode("Typos in ${key}");
+ $bug_message = rawurlencode("The key '${key}' in '${search_options['repo']}' channel is translated as:\n\n'${target_string}'\n\nand must be\n\n");

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

change 'and must be' by 'and should be'

@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

change 'and must be' by 'and should be'

@pascalchevrel

This comment has been minimized.

Show comment Hide comment
@pascalchevrel

pascalchevrel Apr 5, 2013

Owner

It looks excellebt, probably better than what I would have done myself :) I made a few comments for minor issues, please fix them and I will merge that in. Thanks!

Owner

pascalchevrel commented Apr 5, 2013

It looks excellebt, probably better than what I would have done myself :) I made a few comments for minor issues, please fix them and I will merge that in. Thanks!

@tx2z

This comment has been minimized.

Show comment Hide comment
@tx2z

tx2z Apr 6, 2013

Collaborator

All changes done :)

Collaborator

tx2z commented Apr 6, 2013

All changes done :)

pascalchevrel added a commit that referenced this pull request Apr 6, 2013

Merge pull request #53 from tx2z/issue24
Issue24 - Link allowing opening a bug for a typo

@pascalchevrel pascalchevrel merged commit f76733a into mozfr:master Apr 6, 2013

1 check failed

default Scrutinizer: 11 Comments, 0 Changed Files
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment