[go: nahoru, domu]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds non-conflicting default new database name #13099 #13103

Closed
wants to merge 1 commit into from
Closed

Adds non-conflicting default new database name #13099 #13103

wants to merge 1 commit into from

Conversation

Achilles-96
Copy link
Contributor
@Achilles-96 Achilles-96 commented Mar 19, 2017 •
edited
Loading

Signed-off-by: Raghuram Vadapalliraghuram.vadapalli@research.iiit.ac.in

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests

@codecov
Copy link
codecov bot commented Mar 19, 2017 •
edited
Loading

Codecov Report

Merging #13103 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #13103   +/-   ##
=======================================
  Coverage   54.36%   54.36%           
=======================================
  Files         465      465           
  Lines       69685    69685           
=======================================
  Hits        37881    37881           
  Misses      31804    31804

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13225ce...4ca8cfb. Read the comment docs.

@ibennetch
Copy link
Member

Nice work, but I think a better solution is to remove the default value entirely.

@ibennetch ibennetch mentioned this pull request Mar 19, 2017
Closed
@ibennetch
Copy link
Member

Note also that we have #13104 which catches the situation where a user does enter the same database name, so we'll try continue having your pull request address the default values.

nijel
nijel suggested changes Mar 20, 2017
View reviewed changes
Copy link
Contributor
@nijel nijel left a comment

Choose a reason for hiding this comment

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

I'd also prefer not to have default value. Imagine user has databases named db1...db1000, with this patch going to operations tab would lead to 1000 queries just to have default value which almost nobody will use....

@nijel nijel self-assigned this Mar 20, 2017
@Achilles-96
Copy link
Contributor Author

Do you also feel default value must be removed for rename form?

@nijel
Copy link
Contributor
nijel commented Mar 21, 2017

Yes, I think having the field empty by default is best approach.

@Achilles-96 Achilles-96 force-pushed the Issue-13099 branch 2 times, most recently from 7a703cb to 4ca8cfb Compare March 21, 2017 09:13
@Achilles-96
Removes default new database name phpmyadmin#13099
4ca8cfb 
Signed-off-by: Raghuram Vadapalli<raghuram.vadapalli@research.iiit.ac.in>
@nijel nijel added this to the 4.7.0 milestone Mar 21, 2017
@nijel
Copy link
Contributor
nijel commented Mar 21, 2017

Merged as 553d1ff into QA_4_7 branch, thanks for your contribution!

@nijel nijel closed this Mar 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nijel nijel nijel approved these changes

Assignees

@nijel nijel

Labels
None yet
Projects
None yet
Milestone
4.7.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Achilles-96 @ibennetch @nijel