Skip to main content
Topic: IRC (Read 107285 times) previous topic - next topic
0 Members and 3 Guests are viewing this topic.

Re: IRC

Reply #180

Quote(04/27/2013 12:48:28 AM) groundup: "pass to objectify"?

Sorry, lol... I meant, make the rest of controllers object oriented
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #181

(12:26:57 PM) Trekkie101: Norv, We need to figure out how WordPress handles plugins
(12:27:24 PM) Trekkie101: Because it kicks the booty off us, simple install, plugins can even issue updates that you just click "update" on
(12:27:30 PM) Trekkie101: Core patches applied
(12:27:46 PM) Trekkie101: The works, its friggin' beautiful
(12:45:11 PM) groundup: Put files in directory. Find files when you click install. Register classes by reading the start of the name of the class. Extend base class.
(12:46:15 PM) groundup: Could be different but I think that is how they do it. It's why when you create an action that extends the controller, it needs to be in the form Action{ControllerName}Controller or something like that
(12:48:30 PM) groundup: Maybe an event listener in the autoloader? dunno
(10:53:07 PM) groundup: Trekkie101, I really like how easy it is as well :)
The best moment for testing your PR is right after you merge it. Can't miss with that one.

 

Re: IRC

Reply #182

(11:57:04 PM) emanuele: So, when are we going to release a beta?
(11:57:17 PM) emanuele: And don't tell me "when you have fixed all the bugs you wrote!" :P
(11:57:36 PM) ***emanuele didn't drink, but is probably drunk


(11:28:34 PM) groundup: Shouldn't preparseCode() take in to account what BBC you have enabled?
(11:29:34 PM) groundup: *preparsecode() (seems like it should have a _ or camelCase)
(11:30:36 PM) emanuele: I *think* preparsecode is to fix any kind of code and nesting and so on, just to be sure It Works(TM)
(11:32:00 PM) groundup: If I don't have any BBC enabled, it screws up all of the formatting. It also doesn't have any hooks in there for preparsing and 'un'-preparsing code
(11:32:43 PM) emanuele: Interesting...
(11:32:48 PM) emanuele: on any text?
(11:34:42 PM) groundup: Okay, mind you, I haven't tested it, but if you look at Post.subs.php it is pretty straight forward. If I have a post "This is the start of a SMF quote block:
[nobbc][quote]" it will remove that block.
(11:35:06 PM) groundup: Based on: // Trim off trailing quotes - these often happen by accident.
(11:36:29 PM) groundup: Lets say you disabled the
[code] tag. It will close any opened ones even if it is disabled.
(11:36:40 PM) groundup: Pretty much, it tries to close open BBC tags that are disabled.
(11:37:18 PM) groundup: On another note this line should have {} around the following foreach: if (preg_match_all('~(\[(/)*code(?:=[^\]]+)?\])~is', $message, $matches))
(11:39:32 PM) groundup: Hmm... you can't stop it from doing /me to [me] conversion. Admins can always add [html] in there or [html] gets removed if you aren't an admin even if it is disabled
(11:39:38 PM) emanuele: yeah, on that respect it is true: it will fix everything even if not necessary...

(11:40:13 PM) groundup: Can't disable [time] I guess?
(11:40:14 PM) emanuele: though...
(11:40:45 PM) emanuele: why not?
(11:41:15 PM) groundup: What I am thinking is that the tags should have a preparse function and an unpreparse method. If the tag isset() and isn't disabled, run the (un)preparse() method
(11:41:38 PM) groundup: $parts[$i] = preg_replace('~\[time(?:=(absolute))*\](.+?)\[/time\]~ie', '\'[time]\' . (is_numeric(\'$2\') || @strtotime(\'$2\') == 0 ? \'$2\' : strtotime(\'$2\') - (\'$1\' == \'absolute\' ? 0 : (($modSettings[\'time_offset\'] + $user_info[\'time_offset\']) * 3600))) . \'[/time]\'', $parts[$i]); <-- that line
(11:41:46 PM) emanuele: the big issue would arise the moment you disable a tag and then you enable it again after a while
(11:42:04 PM) emanuele: preparsecode is run only at post-time
(11:42:13 PM) groundup: Yeah, I thought about that. It should run the method when you enable/disable tags
(11:42:55 PM) emanuele: ...a lot of hassles...
(11:43:02 PM) groundup: That is obviously a lot of work for the database. Otherwise, it is a bug IMO. Might be a bug that won't be fixed for the benefit of the admin/developer heh
(11:44:44 PM) groundup: I think lines like this make it harder for the poster: $parts[$i] = preg_replace('~\[(black|blue|green|red|white)\]~', '[color=$1]', $parts[$i]);
(11:44:54 PM) groundup: We should just have as a BBC
(11:47:31 PM) emanuele: dunno...that means some useless repetition...
(11:47:47 PM) emanuele: parse_bbc is not for noobs anyway...
(11:49:50 PM) groundup: lol
(11:51:15 PM) groundup: I wonder what happens if you just return; early and don't do any preparsing.
(11:52:05 PM) groundup: The reason I am reporting this is because I am going to have a forum where I don't want any BBC
(11:53:03 PM) emanuele: make sense
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #183

(05/03/2013 12:05:02 AM) groundup: 'SELECT approved' that query should just use getTopicInfo(). At some point getTopicInfo() should have caching on it and extraneous queries like that would be slow
(12:06:05 AM) emanuele: good catch, yes that should go there
(12:06:49 AM) emanuele: but there are many other now that I tried a search...
(12:06:57 AM) groundup: elseif ($posterOptions['id'] != $user_info['id']) the query following that should use getBasicMemberData()

(12:09:29 AM) groundup: Am I missing something or is createPost() doing everything backwards? Add the attachments, then create the message, add the attachments to the message, then create the topic, add the messages to the topic.

(12:10:23 AM) groundup: I need to do a test but I don't think you can do this without a trigger: // @todo Why not set id_msg_modified on the insert?
(12:10:48 AM) groundup: If you can in MySQL, might not be able to in SQLite or Postgres
(12:11:31 AM) emanuele: that comment is there since 2.0, so... O:-)
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #184

(12:12:27 AM) groundup: This should be refactored to a new function in Boards.subs.php: // Increase the number of posts and topics on the board.
(12:13:15 AM) groundup: setBoardPosts($count, $approved = true)

(12:13:18 AM) emanuele: https://github.com/emanuele45/Dialogo/branches/updateAnything
(12:13:38 AM) groundup: What am I looking at?
(12:13:46 AM) emanuele: a mess! :P
(12:14:13 AM) groundup: Since this is part of what I need to do for my application, I might just refactor this
(12:15:20 AM) emanuele: I used as a template the updateMemberData to create several functions updateTopicData, updateBoardData, etc.
(12:16:15 AM) emanuele: https://github.com/emanuele45/Dialogo/commit/c19e40c8c657b34e618f79944809c548e0ccea84
(12:16:34 AM) emanuele: search
(12:16:35 AM) emanuele: https://github.com/emanuele45/Dialogo/commit/c19e40c8c657b34e618f79944809c548e0ccea84
(12:16:48 AM) emanuele:  // Increase the number of posts and topics on the board.
(12:16:52 AM) emanuele: and you'll find it
(12:17:56 AM) groundup: Very nice. PR :P
(12:17:58 AM) emanuele: it is one of my classic crazy refactoring lol
(12:18:24 AM) emanuele: it scared Norv away in a second! :P
(12:20:16 AM) groundup: I am not sure I like abstracting updateTable() that much, but it looks good

(12:21:39 AM) groundup: Maybe if you are going to abstract a table update, you should do it at the database abstraction layer
(12:24:16 AM) groundup: $smcFunc['db_update']($name, array $table, array $col_val = array($col => $val), $options) $options = array(where, limit, sort, limit);
(12:25:27 AM) Norv: Right, it is heading towards a DAL of sorts, and it's very nice
(12:25:29 AM) groundup: if is_array($table) && $cannot_do_multi_table_updates ? foreach ($table as $tbl) {call this function with a single table; return}
(12:25:45 AM) groundup: Same thing with deletes at that point
(12:26:37 AM) groundup: emanuele, I can't wait to grab the diff from that PR ;)
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #185

(11:48:19 PM) groundup: Norv, now that you have OOP controllers, how about a autoloader?
(11:53:42 PM) TimeVortex: __autoload ftw
(11:56:55 PM) groundup: If the actions are standardized in the controller class like BoardIndex_action, then it would be really simple for the dispatcher. The array would be simple $route => $controller; call $controller->{$route}_action

(08:47:26 AM) Norv: >> groundup: If the actions are standardized in the controller class like BoardIndex_action, then it would be really simple for the dispatcher. The array would be simple $route => $controller; call $controller->{$route}_action
(08:47:26 AM) Norv: Yes, this is the purpose. I wasn't in a hurry with it, but yes. Atm not many match the action though.

(05/08/2013 12:02:54 AM) groundup: Which do you think is a better interface for caching: getMulti(array $keys) with get(string $keys) or get(array|string $keys)?
(12:03:38 AM) groundup: With getMulti() get can have a callback method. With just get() we need another method for getWithCallback()
(12:04:02 AM) groundup: I think I will just mimic memcached

(12:27:48 AM) emanuele: Norv, if you pass by: http://www.simplemachines.org/community/index.php?topic=329171.msg3538605#msg3538605
(12:27:58 AM) emanuele: Sooner or later I'll post it at elk too
(12:28:00 AM) emanuele: ;)

(08:51:04 AM) Norv: *blinks and tries to read necroposts LOL*. Yes, inconsistent. Agree. I think.
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #186

(01:21:45 AM) groundup: https://github.com/norv/elkarte/commit/60a36152b1c47e632b97b75bb6ab2eb42009a456#L1R532 I am really interested to know if that is the most efficient method for that.
(01:46:43 AM) groundup: https://github.com/norv/elkarte/blob/7278645528d5d2c3a02d340a9e6375eff9d98e1f/sources/controllers/Topic.controller.php#L116 refactor that as this: require_once SUBSDIR . '/Topic.subs.php'; $topic_info = getTopicInfo($topic); $is_sticky = (bool) $topic_info['is_sticky'];
(01:53:39 AM) groundup: In Topic.subs.php there are a lot of functions with names like messageInfo() where there is no verb there.
(02:01:14 AM) groundup: https://github.com/norv/elkarte/blob/7278645528d5d2c3a02d340a9e6375eff9d98e1f/sources/controllers/Topic.controller.php#L50 another one... why not just get all of the info and extract what you need from that query. SELECT * FROM someTable WHERE id = $id; is a hell of a lot faster when you cache it (even with the query cache) than SELECT col1 FROM someTable WHERE id = $id; SELECT col2 FROM someTable WHERE id = $id; SELECT col3... you

(09:04:24 AM) Norv: >> why not just get all of the info [...]
(09:04:24 AM) Norv: Do it.
(09:05:42 AM) Norv: We have indeed queries simply moved in subs, when in reality they need consolidated, in parametrized functions, and add caching to them
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #187

(02:15:38 AM) groundup: https://gist.github.com/joshuaadickerson/5536962

(07:22:06 AM) groundup: https://gist.github.com/joshuaadickerson/5538182
(07:22:38 AM) groundup: I was thinking that would work as a cache interface that would cover everything. It isn't about getting the least common denominator. It is about getting the most features and faking it for those that can't.
(07:23:49 AM) groundup: Don't have callback support? That's fine, just check to see what it is and then run that callback. Don't have increment? do a get, then increment that value, and set it again. CAS is special and can't be faked, so don't try it. Lists like Redis? Easy, just make sure that it is an array
(07:24:53 AM) groundup: Want to use the database as a cache with a MEMORY table or just have a fast database? The $parameters make it possible to inject the correct values.

(09:02:57 AM) Norv: >> groundup: I was thinking that would work as a cache interface that would cover everything. [...]
(09:02:57 AM) Norv: Dunno... Do we really need to reinvent the wheel here? I would really love it if you would look at Stash, the developer who makes it is obsessed with performance, and as far as I can tell from reading and analyzing some of it (no benchmarking) it's very good.
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #188

(09:41:13 AM) groundup: Norv, can you merge those PRs from you and em?
(09:41:48 AM) Norv: Dunno... Is mine tested? :D
(09:41:55 AM) Norv: O:-)
(09:42:17 AM) groundup: Obviously you didn't test it :P
(09:43:05 AM) Norv: Ema doesn't have a PR now, or?
(09:43:23 AM) groundup: I didn't mean for this commit to get in to that PR: https://github.com/joshuaadickerson/NotElkarte/commit/4c5416b545f2d2abd61990a58c582bf7e7773a64
(09:43:47 AM) groundup: https://github.com/elkarte/Elkarte/pull/206
(09:44:27 AM) groundup: The point of that commit was just a proof of concept.

(09:45:36 AM) groundup: This stash? https://github.com/tedivm/Stash
(09:47:25 AM) Norv: Yes, on Stash
(09:48:16 AM) groundup: Very limited on features
(09:48:42 AM) groundup: Not a lot of drivers either
(09:52:22 AM) groundup: Hmm... I am confused about the Sub drivers. Just realized that was there.
(10:28:23 AM) groundup: I am starting to prefer SELECT * as opposed to SELECT col, col, col
(10:29:09 AM) groundup: 1) it is a hell of a lot less to write 2) it is less network IO 3) it is less chances for the query cache to miss it
(10:29:53 AM) groundup: Disadvantage: you lose naming of columns (which you can still do) and the column order goes to the engine

(12:14:45 PM) groundup: No idea how but all of a sudden my files contain a bunch of crap like: "<<<<<<< HEAD" when I tried to merge that giant commit you just did

(12:20:07 PM) groundup: Any idea how to get rid of all of these "<<<<<<< HEAD" lines?
(12:20:43 PM) Norv: It's because of conflicts. I usually use Meld, it has an awesome 3-way merge view.
(12:21:12 PM) groundup: I apparently just corrupted it. Oh well, the only reason I started with it was because I wanted to get you to merge that PR ;)
(12:21:21 PM) Norv: My suggestion for your master though, is: make another branch with what you have now. To keep it all there. Then,
(12:21:56 PM) Norv: reset --hard your master, to some old commit (i.e. before my PR)
(12:22:06 PM) Norv: then pull master from elkarte repo again
(12:22:31 PM) groundup: ... when I wake up.
(12:22:45 PM) Norv: then, create a branch for your PR (s). And when you're on the new branch, cherry-pick one by one the commits from the "backup branch".
(12:22:56 PM) Norv: Sure :D

(12:23:22 PM) groundup: btw, test those commits I made... I didn't :P
(12:23:27 PM) Norv: :D
(12:23:40 PM) Norv: I'll pick them, no worries.
(12:24:04 PM) groundup: No different from you ;) Post/Poll... That should be one of the first things you test
(12:24:18 PM) Norv:  /hides
(12:24:26 PM) Norv: It wasn't meant to be merged yet!
(12:24:30 PM) groundup: lol
(12:24:42 PM) groundup: but you put it as a PR anyway
(12:25:06 PM) Norv: Only for people to know what I'm playing with! :D
(12:25:19 PM) groundup: The only reason I grabbed the repo is for that one commit that I didn't mean to put in there. lol
(12:25:30 PM) groundup: The one that implements some caching
(12:25:45 PM) Norv: It's interesting but you're right, it should be separate
(12:25:49 PM) Norv: I'll take care of that
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #189

(12:10:15 PM) Norv: Josh, please take a look at all comments on Lab's PR. Also there are a couple (rather big) commits not reviewed at all. I honestly don't have much time to take those meh stuffs. :(
(12:14:07 PM) groundup: Which ones?
(12:14:54 PM) Norv: Oldies, troglodite guy
(12:15:03 PM) groundup: 5:15AM. I need to go to sleep
(12:15:18 PM) groundup: I talked to him about some of them. Most of them are just pedantic BS from Arantor
(12:15:25 PM) groundup: RGB or HEX, who cares?
(12:16:06 PM) Norv: If he doesn't fix pedantic stuff, fine. But it introduces uselessly crap
(12:16:31 PM) groundup: He didn't put two line breaks after a declarative block, woopty doo. That can get fixed later
(12:16:51 PM) groundup: *conditional
(12:17:55 PM) groundup: He needs to learn to take criticism better, sure. Though, he did get lambasted over a few very minor issues. No bugs or major issues there
(12:18:00 PM) Norv: More variables in templates. Hex, it's used almost everywhere. People shouldn't keep switching when they work with the code. Fixed later?
(12:18:27 PM) groundup: Yeah, I talked to him about that. However, before it was a ternary operator. He didn't fix an issue, but he didn't make it worse
(12:18:30 PM) Norv: I don't care about persons. I care about the bs this starts to introduce.
(12:19:09 PM) groundup: I prefer hex, and suggested he change it, but if he doesn't, it won't make a difference.
(12:19:31 PM) groundup: I prefer using a standard.
Last Edit: May 08, 2013, 07:01:20 am by TestMonkey
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #190

 Arantor is not HappyTroglodyte and resents the implication.

I haven't even commented on SMF on Github in months and if I do, it's under my own name.

Re: IRC

Reply #191

Oh, hey.
Don't take it badly. I have actually wondered if you were HT, too, to be honest. With no implication. Sorry about that.

Back to the content of the matter. I don't want to comment too much, nor can I spend much time on.

I will say this though.
For arguably the first time in SMF history, childish egotrips without backup have been the excuse for issues to enter the codebase.
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #192

No, never thought about you Arantor.
Bugs creator.
Features destroyer.
Template killer.

Re: IRC

Reply #193

(07:26:11 PM) emanuele: Norv maybe you already figured it out:
(07:26:13 PM) emanuele: require_once(ADMINDIR . '/Modlog.php');
(07:26:20 PM) emanuele: require_once(SUBSDIR . '/Modlog.subs.php');
(07:26:31 PM) emanuele: in ModerationCenter.controller.php
(07:27:15 PM) ***Norv has lost count of what's where... What is the problem with it?
(07:28:26 PM) emanuele: the first should be replaced by the second :P
(07:28:32 PM) emanuele: it's the only one
(07:30:37 PM) emanuele: I forgot to commit that before you merged the branch, but since you said you were going to fix other things add that one too. ;)
(07:31:16 PM) Norv: Oh ok. I didn't merge your fixes... I think
(07:31:49 PM) ***emanuele is going mad...
(07:31:58 PM) emanuele: I thought I see a merge notice... lol
(07:34:24 PM) Norv: 'twas the cache thing!
(07:34:56 PM) Norv: but if you're happy with it now, we can merge up fixes too. They're great :D
(07:35:46 PM) emanuele: yeah, possible, though I saw the cache one... lol
(07:36:16 PM) emanuele: well, I pushed it
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: IRC

Reply #194

(07:54:21 PM) groundup: Feel free to reject that entire PR
(09:42:38 PM) Norv: groundup: I will pick useful stuff. You can fix your master by a reset or: https://gist.github.com/norv/5542609

(08:11:50 PM) emanuele: How do we fix the admin search?

(08:15:27 PM) groundup: In Load.php - https://gist.github.com/joshuaadickerson/5541980
(08:27:50 PM) groundup: Refresh and look at the comments
(08:33:56 PM) groundup: membergroupsById() in Membergroups.subs.php should be rewritten to use that
(09:14:31 PM) groundup: https://gist.github.com/joshuaadickerson/5542380
(09:33:42 PM) groundup: cache_getMembergroupList() in Membergroups.subs.php should be removed in favor of using this with some filtering.
(09:45:07 PM) groundup: That stuff that I just gist'd is done from a copy of Elkarte that is for a completely different project.

(05:14:17 AM) Norv: groundup: re: https://gist.github.com/joshuaadickerson/5541980 please feel free to PR the proposal.

(07:39:09 PM) groundup: Norv, that means I should test it and I should have a copy running. Too much work :P
(08:18:26 PM) groundup: Damn, I forgot to switch branches after I created it.

(08:44:02 PM) Norv: groundup: cool! As for branches. Make another branch from this; PR that branch; reset your master. Safer I'd think. Once you get used to git, there are about a dozen ways for these small thingies
(08:57:12 PM) Norv: As for 'too much work', lol to you. You have the opportunity to align your needs with improving Elk; make it suit you better. Yes, by doing the work. We're relaxed on procedures (well we dont have almost any lol), but don't take for granted untested PRs.
The best moment for testing your PR is right after you merge it. Can't miss with that one.