preinheimer / xhprof

XHGUI is a GUI for the XHProf PHP extension, using a database backend, and pretty graphs to make it easy to use and interpret.
Other
833 stars 185 forks source link

Changed server_id size from 3 to 17 #102

Closed tperalta82 closed 4 years ago

tperalta82 commented 7 years ago

Schema change due to #99

aik099 commented 7 years ago

Rebasing and squashing is needed.

tperalta82 commented 7 years ago

Yeah I noticed, it doesn't make any sense at all, before pushing & commiting , I tried with different editors, from gedit to pluma to vi / nano / netbeans, something is reallty wrong , well, reject all the pr's ... again, as this doesn't make any sense.

I'll redo this when I have the time to reset the repo and start the modifications from scratch

On Tue, Oct 18, 2016 at 4:37 PM, Alexander Obuhovich < notifications@github.com> wrote:

@aik099 commented on this pull request.

In xhprof_lib/utils/Db/Mssql.php https://github.com/preinheimer/xhprof/pull/102:

  • CREATE TABLE dbo.details
  • (
  • id nchar(17) NOT NULL,
  • url nvarchar(255) NULL DEFAULT NULL,
  • c_url nvarchar(255) NULL DEFAULT NULL,
  • timestamp datetime NOT NULL DEFAULT getdate(),
  • [server name] nvarchar(64) NULL DEFAULT NULL,
  • perfdata nvarchar(max) NULL,
  • type smallint NULL DEFAULT NULL,
  • cookie nvarchar(max) NULL,
  • post nvarchar(max) NULL,
  • get nvarchar(max) NULL,
  • pmu int NULL DEFAULT NULL,
  • wt int NULL DEFAULT NULL,
  • cpu int NULL DEFAULT NULL,
  • server_id nchar(17) NOT NULL DEFAULT N't11',

The problem comes from mass-replacement of line endings from \r\n into \n it seems.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/preinheimer/xhprof/pull/102, or mute the thread https://github.com/notifications/unsubscribe-auth/AEGffu4v8fuBFn8kN1S7rbtS9akkOvunks5q1OfNgaJpZM4KVFNd .

aik099 commented 7 years ago

PR does make sense, but it needs to be fixed. Fixing isn't really that hard. No need to close PR and sending it again. Just change existing PR branch.

flip111 commented 4 years ago

@tperalta82 your PR looks good. I'm running into this issue now too.

aik099 commented 4 years ago

This was done in #122.