Page MenuHomePhabricator

No way to kick users out of a Conpherence
Closed, ResolvedPublic

Description

If you can edit a Conpherence, you should be able to kick users out of it.

Event Timeline

epriestley raised the priority of this task from to Low.
epriestley updated the task description. (Show Details)
epriestley added a project: Conpherence.
epriestley added a subscriber: epriestley.

u wot m8 ill +1 u

then implement this task

then kick u outta every room

better watch out m8

chad claimed this task.Oct 5 2015, 8:55 PM
CodeMouse92 added a subscriber: CodeMouse92.

I'd like to contribute and help in finishing this. Where do I go?

chad added a comment.Feb 2 2016, 6:17 PM

This isn't a good task to learn how to contribute to Phabricator.

Why is that?

chad added a comment.Feb 2 2016, 6:56 PM

The engineer who built Conpherence is no longer contributing code, which means another developer would have to learn it first to give you pointers on where to start. This isn't a great use of our time. I expect Conpherence to get overhauled sometime in the next 3-6 months.

chad added a comment.Feb 2 2016, 7:06 PM

But maybe you are an adventurous soul! Code is reasonably organized by application, so src/applications/conpherence/ contains PHP and webroot/rsrc/js/application/conpherence/ contains the JavaScript.

CodeMouse92 added a comment.EditedFeb 2 2016, 7:10 PM

Right about now, it might be worth mentioning that my company instituted a standard of commenting every single logical statement in code, stating the intent of the logical statement (not just restating the code).

Contrary to what one might expect, it actually saves time by cutting down on bugs, making the code easier for other coders to read and grok, and helping the coder keep track of their own logic and train of thought. (We came up with the standard while taking apart a 10-year-old uncommented pile of spaghetti code. In that scenario, we would also mark when we did not understand the intent of some code, and then fill it in later when we figured it out.)

Might that approach be helpful here, if @exp10r3r begins learning Conpherence's code?

chad added a comment.Feb 2 2016, 7:26 PM

There are two main issues. First being we prefer not to context switch ourselves to whatever the issue-du-jour is. That is, we're a very small team (2 people) on a very large project. We maintain throughput by being focused on the current task. We're not currently focused on Conpherence.

The second issue is we don't like writing things twice. We could fix Conpherence bugs today, or if we just wait a few months until Conpherence is the main focus of our development, then they'll get resolved then with more modern components. For example the "widgets" area in Conpherence will be completely removed.

Conpherence, as built, is fairly unique in how most everything else works in Phabricator. It doesn't make much sense planning-wise to continue to patch an old system when a newer one is coming down the pipe.

chad added a comment.Feb 2 2016, 7:29 PM

For the record I did try to do this myself with my limited eng skills, and I gave up. If you're a natural with JavaScript, it's likely an easy task. I'm not.

CodeMouse92 added a comment.EditedFeb 2 2016, 7:30 PM

I'd have to agree not to write things twice. That said, I do know that when we undertook starting our own animation system, we first dismantled the open source one we were considering prior (that's where we commented everything), and figured out the inherent problems at its core, which in turn prevented making the same mistakes in our system. Plus, some approaches may be at least partially salvageable for v2, tying back in with the basic idea of "don't code things twice".

If someone, anyone, commented through Conpherence v1, it would provide a means for anyone else in the future to implement v2. If v2 is similarly commented in its writing, it can be maintained by anyone fairly easily.

chad added a comment.Feb 2 2016, 7:31 PM

In general @epriestley writes more documentation than any engineer I've ever met. At least, most abstractions are well documented.

CodeMouse92 added a comment.EditedFeb 2 2016, 7:33 PM

I'll admit, his docs are quite impressive. What I'm referring to is effectively inline-documentation of the code's intent itself, which is inherently separate from docs, and is solely to make maintenance by any programmer who takes the code on possible, without involving the previous dev in person, and largely without invoking the consequences of Brooks' Law.

The standard (we call it Commenting Showing Intent, or CSI) basically establishes 'why are we looping through all the elements in the array?' or 'why are we using an array at all, vs. a database?'

chad added a comment.Feb 2 2016, 7:35 PM

I'm referring to that as well. Diviner builds docs from his comment blocks. Most of it is only on abstractions though like libphutil.

CodeMouse92 added a comment.EditedFeb 2 2016, 7:36 PM

Right, yeah. CSI is somewhat separate, since most of it isn't intended to be picked up by autodoc tools, but it is also designed to not conflict with those. (I should have the published standard up on my site in the next week.) Docs are "what does the code do" and "how do I use it", CSI is "why is the code like this and what is it doing?"

I'm on the freenode IRC channel. Studying the code now. If I need any help and assuming I don't give up in between, I'll ask my questions.
And as for this thread, I think I've understood both your points.
Being an admirer of Phab's current documentation, I'll try imitating it in my work as well.

I tried doing an arc diff to submit my patch but ran into this,

[2016-02-02 21:15:55] EXCEPTION: (PhutilTypeExtraParametersException) Got unexpected parameters: standard at [<phutil>/src/parser/PhutilTypeSpec.php:150]
arcanist(), phabricator(head=master, ref.master=3264d9f680cb), phutil()
  #0 PhutilTypeSpec::checkMap(array, array) called at [<arcanist>/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php:90]
  #1 ArcanistConfigurationDrivenLintEngine::buildLinters() called at [<arcanist>/src/workflow/ArcanistLintersWorkflow.php:44]
  #2 ArcanistLintersWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]

I could run through it by removing the following section in .arclint file,

,
    "xhpast": {
      "type": "xhpast",
      "include": "(\\.php$)"
    }

But it wouldn't let me commit.

I tried doing an arc diff to submit my patch but ran into this,

[2016-02-02 21:15:55] EXCEPTION: (PhutilTypeExtraParametersException) Got unexpected parameters: standard at [<phutil>/src/parser/PhutilTypeSpec.php:150]
arcanist(), phabricator(head=master, ref.master=3264d9f680cb), phutil()
  #0 PhutilTypeSpec::checkMap(array, array) called at [<arcanist>/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php:90]
  #1 ArcanistConfigurationDrivenLintEngine::buildLinters() called at [<arcanist>/src/workflow/ArcanistLintersWorkflow.php:44]
  #2 ArcanistLintersWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]

I could run through it by removing the following section in .arclint file,

,
    "xhpast": {
      "type": "xhpast",
      "include": "(\\.php$)"
    }

But it wouldn't let me commit.

Update Arcanist

epriestley moved this task from Backlog to v4 on the Conpherence board.Feb 16 2016, 1:41 PM
epriestley edited projects, added Conpherence (v4); removed Conpherence.
epriestley moved this task from Backlog to Broken Stuff on the Conpherence (v4) board.

I tried doing an arc diff to submit my patch but ran into this,

[2016-02-02 21:15:55] EXCEPTION: (PhutilTypeExtraParametersException) Got unexpected parameters: standard at [<phutil>/src/parser/PhutilTypeSpec.php:150]
arcanist(), phabricator(head=master, ref.master=3264d9f680cb), phutil()
  #0 PhutilTypeSpec::checkMap(array, array) called at [<arcanist>/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php:90]
  #1 ArcanistConfigurationDrivenLintEngine::buildLinters() called at [<arcanist>/src/workflow/ArcanistLintersWorkflow.php:44]
  #2 ArcanistLintersWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]

I could run through it by removing the following section in .arclint file,

,
    "xhpast": {
      "type": "xhpast",
      "include": "(\\.php$)"
    }

But it wouldn't let me commit.

Update Arcanist

I've done and updated the diff accordingly. D15168