Skip to content

Conversation

@Tithugues
Copy link
Contributor

@Tithugues Tithugues commented Oct 31, 2019

When partition names starts with a number, the parser wasn't reading the full name and stopped before the first "_".

When partition names contain "_", the parser wasn't reading the full name, but stopped before the first "_".

Signed-off-by: Hugues Peccatte <hugues.peccatte@aareon.fr>
@williamdes
Copy link
Member

Can you please add a test case?

Btw we merge bug fixes on QA branch, so if you want to rebase :)

@williamdes williamdes self-assigned this Nov 1, 2019
@Tithugues Tithugues changed the base branch from master to QA November 1, 2019 08:33
@Tithugues
Copy link
Contributor Author

Sorry, I created the PR on the wrong branch, but the commit were done on QA.

I'll see to update the unit tests.

@ibennetch
Copy link
Member

ibennetch commented Nov 1, 2019 via email

@Tithugues
Copy link
Contributor Author

@williamdes @ibennetch , test case is done.

Is this OK?

@Tithugues
Copy link
Contributor Author

I saw the failure in the unit test! I'll fix it and run the whole tests, not only the PartitionDefinitionTest!

Signed-off-by: Hugues Peccatte <hugues.peccatte@aareon.fr>
@Tithugues
Copy link
Contributor Author

Should be better now...

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

@Tithugues I really do appreciate your work and the test you added ! 💯

@williamdes williamdes added this to the 4.3.3 milestone Nov 1, 2019
williamdes added a commit that referenced this pull request Nov 1, 2019
Fixes: phpmyadmin/phpmyadmin#13951
Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this pull request Nov 1, 2019
Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes williamdes merged commit ccb20a1 into phpmyadmin:QA Nov 1, 2019
@Tithugues Tithugues deleted the feature/fix-13951-partition branch November 1, 2019 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants