akshay.singh → created an issue.
@divya.sejekan,
what’s the issue with MR?
Can you please specify?
If you are talking about the above test failure then it hasn’t failed for in the next test.
Thanks
akshay.singh → changed the visibility of the branch 3422917-paragraphs-description-text to active.
akshay.singh → changed the visibility of the branch 3422917-paragraphs-description-text to hidden.
@vishal-lnwebworks,
you shouldn’t mark the issue Fixed as that is need to be done by the project maintainers once they merged the code.
secondly I have already created an MR for same no need to create a patch for the same work which is done already and also naming conventions for patch is wrong.
If the above MR code seems right to you then you can mark it as RTBC.
Setting back to needs review.
Let others make a review and mark it as RTBC so that maintainer can proceed further.
Thanks
akshay.singh → changed the visibility of the branch 3422917-paragraphs-description-text to active.
akshay.singh → changed the visibility of the branch 3422917-paragraphs-description-text to hidden.
akshay.singh → created an issue.
I am working on it.
apaderno → credited akshay.singh → .
+1 for RTBC
MR created.
Thanks
MR created.
Please review.
Thanks
akshay.singh → created an issue.
so we have two options either to add core
or we can restrict core_version_requirement
to ^8.8 || ^9 || ^10
.
I have choosed the core one as not to remove support for D8 projects.
Please review created an MR.
Thanks
akshay.singh → made their first commit to this issue’s fork.
I have tried to simplified the logic and keeping in mind the error and created an MR for same.
please review.
Thanks
akshay.singh → made their first commit to this issue’s fork.
akshay.singh → made their first commit to this issue’s fork.
Created an MR with the fixes
+1 for RTBC
Thanks
hello @satish_kumar,
there is no need to attach the screenshots and again the patch which was attached for review.
akshay.singh → made their first commit to this issue’s fork.
created MR and updated the issue summary.
Please review.
Thanks
changes done MR created
please review
Thanks
akshay.singh → created an issue.
I have reviewed the patch and created a MR for same.
+1 RTBC
Thanks
akshay.singh → made their first commit to this issue’s fork.
Hello, and a warm welcome to the Drupal community!
You don’t need confirm role to contribute patches or modules to drupal.org
The 'confirmed' role is for users that contribute to this website. In this case, you've not contributed any content except this post, so there is no content to review.
Postponing for now, after you have posted some content on Drupal.org. You can add a comment to this issue to request a new review in order to get 'confirmed' or you will get that automatically.
Please visit the Become a confirmed user → page for information.
Here is a list of resources that will assist you in making helpful contributions:
point #6 addressed and MR created
Please review
Thanks
akshay.singh → made their first commit to this issue’s fork.
All points incorporated.
Please review.
Thanks
+/**
+ * @file
+ * Module autogrow_textarea.
+ */
this is not the right info for the file.
/**
* Implements hook_element_info_alter().
*
* Adds autogrow_textarea.js as attached JavaScript to each textarea.
*/
these are the generic hooks not specific to piece of code, one shouldn't add this in function doc it must be added as commenting.
new_3374628.patch
not the right naming convention for a patch.
addressing #7
akshay.singh → made their first commit to this issue’s fork.
@el7cosmos,
Please review
Thanks
akshay.singh → made their first commit to this issue’s fork.
akshay.singh → made their first commit to this issue’s fork.
After applying the above patch, I still got some leftover issues which i have fixed and provided an interdiff as well.
Raised MR for all the fixes, except the readme 84 | WARNING | Line exceeds 80 characters; contains 116 characters I think which we can ignore.
Please review.
Thanks
FILE: /Users/akshaysingh/projects/dcontri/fast_404-3380617/tests/src/Unit/Fast404EventSubscriberTest.php
-----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------------
6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\fast404\EventSubscriber\Fast404EventSubscriber.
-----------------------------------------------------------------------------------------------------------------------------------------------
FILE: /Users/akshaysingh/projects/dcontri/fast_404-3380617/README.txt
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
84 | WARNING | Line exceeds 80 characters; contains 116 characters
116 | WARNING | Line exceeds 80 characters; contains 81 characters
----------------------------------------------------------------------
FILE: /Users/akshaysingh/projects/dcontri/fast_404-3380617/src/Fast404.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------------
6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Database\Database.
293 | ERROR | [x] Expected 1 blank line after function; 0 found
294 | ERROR | [x] The closing brace for the class must have an empty line before it
-------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------
FILE: /Users/akshaysingh/projects/dcontri/fast_404-3380617/src/EventSubscriber/Fast404EventSubscriber.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AND 3 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Symfony\Component\HttpKernel\Event\ExceptionEvent.
30 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found
31 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found
32 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found
33 | ERROR | [x] Expected 3 space(s) before asterisk; 2 found
41 | ERROR | [ ] Missing parameter comment
53 | WARNING | [x] Inline @var declarations should use the /** */ delimiters
53 | WARNING | [x] There must be no blank line following an inline comment
53 | WARNING | [ ] There must be no blank line following an inline comment
----------------------------------------------------------------------------------------------------------------------------------------------
MR 64 is resolving the issues.
+1 RTBC
Thanks for working
@rob holmes,
I can't see one on my account page. See
here →
Oh so the maintainer merged the code but didn't marked it fixed and didn't provided the credit.
Thanks @anybody
@anybody
How come you marked this fixed as you are not the maintainer? or by mistake?
I am not able to reproduce it, and as the fields are required with the predefined values. I can't think of the null case.
Closing for now.
akshay.singh → made their first commit to this issue’s fork.
+1 for RTBC
changes done please check.
Thanks
After applying the above patch, I still got these errors which i have fixed and raised MR and attached a interdiff also.
ILE: codefilter-3372646/tests/src/Unit/CodeFilterTest.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------
6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\codefilter\Plugin\Filter\CodeFilter.
------------------------------------------------------------------------------------------------------------------------------------
FILE: codefilter-3372646/src/Plugin/Filter/CodeFilter.php
-----------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------
6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\Html.
-----------------------------------------------------------------------------------------------------------------------
I am working on this
akshay.singh → created an issue.
Changes done requested and MR.
Please review.
Thanks
akshay.singh → created an issue.
akshay.singh → created an issue.
Please review.
Changes are done and MR has been created.
Thanks
after applying the patch, still there are some left over issues.
PFA
I am working on those and will raise a MR with changes.
FILE: trumba/trumba.module
--------------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
--------------------------------------------------------------------------------------------------------------------------------
9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Cache\CacheableMetadata.
31 | ERROR | [x] Parameter comment indentation must be 3 spaces, found 4 spaces
33 | ERROR | [x] Parameter comment indentation must be 3 spaces, found 4 spaces
58 | ERROR | [x] Short array syntax must be used to define arrays
62 | WARNING | [x] A comma should follow the last multiline array item. Found: ''
--------------------------------------------------------------------------------------------------------------------------------
FILE: trumba/trumba.links.menu.yml
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
6 | ERROR | [x] Expected 1 newline at end of file; 2 found
-------------------------------------------------------------------------------
FILE: trumba/trumba.routing.yml
----------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------
9 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------------
FILE: trumba/src/Plugin/Block/TrumbaBlockBase.php
---------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------
6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Block\BlockBase.
---------------------------------------------------------------------------------------------------------------------
Please review
I have pushed the changes.
and there is no need to use t() with logger.
Thanks
I am working on it.
Updated the issue summary.
Hey @t.maquin,
I have done that and it is possible. I can see from your code that you have already injected some of the services through arguments which is the right way to do that. What is the trouble here?
Hey @geek-merlin,
Thanks for the commit but you forgot to mark the issue fixed and give the credits.
@lpeidro,
Reporter must not mark the status as RTBC. Only the reviewers is allowed to do so.
Thanks
I have fixed the rest of the issues.
Remaining tasks
FILE: /var/www/html/dcontri/linked_field/src/LinkedFieldManagerInterface.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 2 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
13 | WARNING | The class short comment should describe what the class does and not simply repeat the class name
55 | ERROR | The text '@deprecated No longer used by internal code and not recommended.' does not match the standard format: @deprecated
| | in %deprecation-version% and is removed from %removal-version%. %extra-info%.
55 | ERROR | Each @deprecated tag must have a @see tag immediately following it
----------------------------------------------------------------------------------------------------------------------------------------------
akshay.singh → created an issue.
I have created the MR.
Please review.
Thanks
akshay.singh → created an issue.
https://www.drupal.org/project/file_mime_validator/releases/8.x-2.0 →
supports D9 & D10 with new features.
Usually, after reviewing a project, we allow the developer to opt projects into security advisory coverage.
This project is too small for us and it doesn't contain enough PHP code to really assess your skills as developer.
Have you made any other contributions that we could instead review?
You can request apaderno → . He can do so.
$payload = [
"model" => $model,
"prompt" => $prompt_text,
"temperature" => (int) $temperature,
"max_tokens" => (int) $max_tokens,
];
$json_input = json_encode($payload);
$headr = [];
$headr[] = 'Content-Type: application/json';
$headr[] = 'Authorization: Bearer ' . $access_token;
// cURL starts.
$curl = curl_init();
curl_setopt($curl, CURLOPT_URL, $url);
curl_setopt($curl, CURLOPT_POST, 1);
curl_setopt($curl, CURLOPT_POSTFIELDS, $json_input);
curl_setopt($curl, CURLOPT_HTTPHEADER, $headr);
curl_setopt($curl, CURLOPT_RETURNTRANSFER, TRUE);
$reply = curl_exec($curl);
$http_status = curl_getinfo($curl, CURLINFO_HTTP_CODE);
curl_close($curl);
// cURL ends
$decoded_data = json_decode($reply);
// Error handling for OpenAI API.
if ($http_status != 200) {
$error_msg = $decoded_data->error->type.": ".$decoded_data->error->message;
$ajax_response->addCommand(new HtmlCommand('#chatgpt-result', $error_msg));
return $ajax_response;
}
else {
// processing success response data.
$text = $decoded_data->choices[0]->text;
$ajax_response->addCommand(new HtmlCommand('#chatgpt-result', $text));
return $ajax_response;
}
you must replace curl
with Drupal::httpClient
as it is the preferred and recommended way when it comes to drupal.
Manual Findings:
- Fix these typos
Cridential
,Pintrest
-
public function __construct(ConfigFactoryInterface $config) { parent::__construct($config); $this->config = $config->get('config.facebook_social_feeds_block'); }
you don't need to override the function, as you will get the
$this->config('')
in the build form itself. Please remove it from all the config forms. -
$fbfeeds = "https://graph.facebook.com/" . $fb_app_name . "/feed?access_token=" . $fb_app_id . "|" . $fb_secret_id . '&fields=link,message,description&limit=' . $fb_no_feeds; $response = $this->httpClient->get($fbfeeds, ['headers' => ['Accept' => 'text/plain']]); $url = "https://api.instagram.com/v1/users/self/media/recent/?access_token=" . $insta_access_token . '&count=' . $insta_pic_counts; $response = $this->httpClient->get($url, ['headers' => ['Accept' => 'text/json']]); $request = $response->getBody()->getContents(); $url = "https://api.pinterest.com/v3/pidgets/users/" . $pintrest_user_name . "/pins"; $response = $this->httpClient->get($url, ['headers' => ['Accept' => 'text/json']]); $feeds = $response->getBody()->getContents();
all these client requests must be in the
try
catch
blocks and all theexceptions
must be logged so that admin can know what is broken and a message for the end user. -
remove
#
fromsocial_feeds_block.routing.yml
as they seems odd.
sutharsan,
@gisle is right machine name’s can’t be changed.
But I would you to take backup of your project, create a new post with your preferred name and the module code.
Then ask the moderators to delete the one via email or through the issue queue.
Thanks
Hi apmsooner,
# Information added by Drupal.org packaging script on 2023-01-10
version: '8.x-1.1'
project: 'file_mime_validator'
datestamp: 1673357950
I thought you were saying about this.
And yes that was the intent to stick it for D8 only. As D8 is already EOL.
I will try to give a release for the 8.x-2 in this week which will support d9 & d10.
Hi apmsooner,
- you are getting this #Information because you are downloading the module if you don't want that. You should prefer the recommended
method which is through composer. - currently this dev release → supports for d9 & d10, which I have mentioned on the project page as well. So if you need you can use it for now, soon I will be providing stable release
Thanks