Skip to content
This repository has been archived by the owner on Jun 7, 2024. It is now read-only.

Ask feedback to author of community PRs #93

Merged
merged 4 commits into from
Oct 13, 2020

Conversation

atomiix
Copy link
Contributor

@atomiix atomiix commented Jun 26, 2020

Questions Answers
Description? This new feature will add a comment with a link to a survey once a PR from the community has been merged to get the author's feedback. It will happen once per month max.
Type? new feature
BC breaks? yes
Deprecations? no
Fixed ticket? Partialy Fixes PrestaShop/prestashop-retro#67
How to test? Tests should passes

@@ -58,7 +71,7 @@ public function checkForTableDescription(PullRequest $pullRequest)
$bodyParser = new BodyParser($pullRequest->getBody());

$validationErrors = $this->validator->validate($bodyParser);
if (count($validationErrors) > 0) {
if (\count($validationErrors) > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Move to use use function count

Copy link
Contributor Author

Choose a reason for hiding this comment

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

php-cs-fixer won't accept it I already tried ;) Do you think I should change the rule though?

Copy link
Member

Choose a reason for hiding this comment

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

Ping @PrestaShop/prestashop-core-developers

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason of adding the backslash here? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make php-cs-fixer happy, that's one of its rules

Copy link
Member

@Progi1984 Progi1984 left a comment

Choose a reason for hiding this comment

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

Some changes

@matks
Copy link
Contributor

matks commented Aug 17, 2020

@atomiix what is the status of this PR ? still valid ? or stale ?

@atomiix
Copy link
Contributor Author

atomiix commented Aug 17, 2020

@matks still valid!

@Progi1984
Copy link
Member

@matks still valid!

And review too ;)

@atomiix atomiix closed this Sep 2, 2020
@atomiix atomiix reopened this Sep 2, 2020
@LouiseBonnard
Copy link

Hi @atomiix, is there some work left to do before merging this PR?

@atomiix
Copy link
Contributor Author

atomiix commented Sep 28, 2020

@LouiseBonnard I had to rebase it. Now waiting for reviews ;)

$since = (new DateTime())->sub(DateInterval::createFromDateString('30 days'));
}
$query = 'repo:'.$this->repositoryOwner.'/'.$this->repositoryName.' is:pr is:merged author:'.$mergedFrom.' commenter:'.$commentedBy.' merged:>='.$since->format('Y-m-d');
$search = '{
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of string should be store in a file 🤔
(Can be done in a new PR)

@Progi1984
Copy link
Member

@atomiix Need a rebase

@matthieu-rolland matthieu-rolland merged commit 5be6ff6 into PrestaShop:master Oct 13, 2020
@matthieu-rolland
Copy link
Contributor

thanx @atomiix !

@atomiix atomiix deleted the ask-feedback branch October 13, 2020 09:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add bots to improve productivity and engagement
6 participants