Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

[refactor] introduce debug classes under new structure #213

Merged
merged 7 commits into from
Jun 19, 2020

Conversation

georgehristov
Copy link
Collaborator

Split of #196

Part I - Refactoring of Debug proxy connections

@georgehristov georgehristov force-pushed the feature/organize-debug-structure branch from b847995 to ca0f370 Compare June 19, 2020 10:23
@georgehristov georgehristov force-pushed the feature/organize-debug-structure branch from 4376ae7 to 7d05954 Compare June 19, 2020 10:27
@@ -4,101 +4,9 @@

namespace atk4\dsql;

class Connection_Counter extends Connection_Proxy
/**
* @deprecated will be removed in v2.3
Copy link
Member

Choose a reason for hiding this comment

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

I propose to write here: "will be removed in 2020-dec"

It is much easier for users to know when it will be removed and when to remove it in the code.

(update in everywhere)

@mvorisek
Copy link
Member

Seems ok to merge except one thing - Driver vs. Connection.

Why is driver really that much better?

Driver is usually used for mysqli, PDO, so Connection seems here ok for me to keep and to keep all consistency.

Even if really needed, I propose to keep Connection for now and rename everything at once after the new structure is set. Why? It is much easier to refactor purely NS in user's projects and later (after another consistent release) refactor the short name.

@georgehristov ok with you?

@georgehristov georgehristov requested a review from mvorisek June 19, 2020 11:53
@georgehristov georgehristov force-pushed the feature/organize-debug-structure branch from 782dfd1 to a7b392b Compare June 19, 2020 12:22
@mvorisek
Copy link
Member

mvorisek commented Jun 19, 2020

Then feel free to rename NS of other engines. But please, do not copy from old PR, only minimalistic rename, I will check (still have to compare manually because of old classes are not removed completely so git does handle this as renames).

@georgehristov
Copy link
Collaborator Author

I would keep this PR only for the Debug connections.
You are right that the more concentrated is the PR the easier to understand and accept.
Refactoring of NS structure will follow in another PR.

@georgehristov georgehristov requested a review from mvorisek June 19, 2020 12:35
@georgehristov
Copy link
Collaborator Author

@mvorisek done so pls approve for the record. I will not merge

Copy link
Member

@mvorisek mvorisek left a comment

Choose a reason for hiding this comment

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

Pure NS updates/renames are done well. Approving rename of Debug Connections. No unresolved feedback left.

Please rename freshly (do not reuse anything from changed code from your huge PR) other engines.

@mvorisek mvorisek removed the RTM label Jun 19, 2020
@mvorisek
Copy link
Member

@georgehristov I checked the changelog after rel 2.1.0 and we can merge this right away. If ok with you, merge this PR and create another one for the pure renames (with BC classes like here). @romaninsh is slow, we can merge all renames today today.

@mvorisek mvorisek added the RTM label Jun 19, 2020
@atk4 atk4 deleted a comment from codecov bot Jun 19, 2020
@georgehristov georgehristov merged commit 649836e into develop Jun 19, 2020
@georgehristov georgehristov deleted the feature/organize-debug-structure branch June 19, 2020 12:51
@georgehristov
Copy link
Collaborator Author

Good. Moving on to next task now ...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants