Page MenuHomePhorge
Feed All Stories

Wed, Nov 15

valerio.bozzolan added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

I can also propose another small change to adopt a sigil for the button label, so we don't need an ID.

Wed, Nov 15, 06:58

Tue, Nov 14

valerio.bozzolan added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

We need more help on this. Never approved anything similar, I don't have enough experience.

Tue, Nov 14, 22:12
valerio.bozzolan updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Tried to add a lovely icon but still keeping the label toggling and the whole button selectable

Tue, Nov 14, 22:10
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Renamed butt to btn as per PR. Much more common abbrieviation!

Tue, Nov 14, 20:37
speck accepted D25421: Audit Feed: less verbose when the author is the committer.

Minor, just suggestion

Tue, Nov 14, 19:45
valerio.bozzolan awarded D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup a Love token.
Tue, Nov 14, 16:40
valerio.bozzolan added inline comments to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Tue, Nov 14, 16:40
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Happy to take your suggestions with a couple of changes

  • Remove index from the config labels - we've made collapsible-container.js generic, and not just for an index.
  • The reason for the if/else block was to enable long variable/attribute names so as to have self-documenting code. I've done away with it, but we do need a comment instead so as to show why we associate the 'push this to close' text with the open state and vice-versa.
Tue, Nov 14, 15:33
valerio.bozzolan added a comment to D25436: Update PhutilCowsay.php to work for small cows.

I'm a bit confused (!) I thought there were no PHP changes on Rev 1414 and I approved it, but instead I now see that we have again PHP changes. It's unclear to me whenever the Diff I approved was "updated to reflect changes", or maybe I'm just totally crazy because of real life. Please share your opinion.

Tue, Nov 14, 15:31
Sten closed D25436: Update PhutilCowsay.php to work for small cows.
Tue, Nov 14, 15:05
Sten committed rARC5bc53cfe53d0: Update PhutilCowsay.php to work for small cows.
Update PhutilCowsay.php to work for small cows
Tue, Nov 14, 15:05
waldyrious updated the task description for T15644: Support backticks in inline and block code by wrapping with multiple backticks.
Tue, Nov 14, 14:47Feature Requests, Remarkup
waldyrious updated the task description for T15290: vscode extension for working on phorge codebase.
Tue, Nov 14, 14:44Phorge Development Tools, Phactory: Community Projects
waldyrious added projects to D25466: Improve command line prompts in setup issue pages: UX, Config.
Tue, Nov 14, 14:40Config, UX
waldyrious updated the diff for D25473: Change some instances of "phabricator" to "phorge".
  • Use dynamically generated platform path where possible
Tue, Nov 14, 14:37
waldyrious added inline comments to D25473: Change some instances of "phabricator" to "phorge".
Tue, Nov 14, 14:28
valerio.bozzolan moved T15282: Fix/avoid/simplify similar fatal: detected dubious ownership in repository at '/var/www/phorge' from PingDeath 馃寶 to 馃敟 Trap on the User-valerio.bozzolan board.
Tue, Nov 14, 13:55User-valerio.bozzolan
valerio.bozzolan edited Description on Blessed GitHub phorgelabs Admins.
Tue, Nov 14, 13:55
valerio.bozzolan closed T15206: Clarify administrators of GitHub https://github.com/phorgelabs, a subtask of T15037: Support OAuth login via GitHub/Google/etc?, as Resolved.
Tue, Nov 14, 13:54Governance
valerio.bozzolan closed T15206: Clarify administrators of GitHub https://github.com/phorgelabs as Resolved.

Hoping to be useful we can self-document this joining this group:

Tue, Nov 14, 13:54User-valerio.bozzolan, Governance
valerio.bozzolan created Blessed GitHub phorgelabs Admins.
Tue, Nov 14, 13:53
valerio.bozzolan moved T15163: Auto-Attach a File when dropping it as Task cover image from Backlog to Code Sprint Candidate on the User-valerio.bozzolan board.
Tue, Nov 14, 13:46Affects-Wikimedia, Maniphest, User-valerio.bozzolan, Cover Image, Workboard
valerio.bozzolan added inline comments to D25473: Change some instances of "phabricator" to "phorge".
Tue, Nov 14, 13:37
valerio.bozzolan added a comment to D25421: Audit Feed: less verbose when the author is the committer.

(This was already approved but I wait for another "Yeah land" since I changed a thing)

Tue, Nov 14, 13:23
valerio.bozzolan added a comment to T15669: Document how to install javelinsymbols.

To fix this error:

Tue, Nov 14, 12:34
valerio.bozzolan created T15669: Document how to install javelinsymbols.
Tue, Nov 14, 12:32
waldyrious added a comment to D25466: Improve command line prompts in setup issue pages.

Btw, there are some similar commands in src/applications/config/check/PhabricatorManualActivitySetupCheck.php. Should I change them as well? How can I "break Phorge" to see this page rendered?

Tue, Nov 14, 12:24Config, UX
waldyrious requested review of D25473: Change some instances of "phabricator" to "phorge".
Tue, Nov 14, 12:12
speck planned changes to D25276: Add support for secure connections to the database.

I think there鈥檚 still a chunk to do here, including additional documentation during setup

Tue, Nov 14, 12:01
waldyrious updated the test plan for D25466: Improve command line prompts in setup issue pages.
Tue, Nov 14, 11:50Config, UX
waldyrious updated the diff for D25466: Improve command line prompts in setup issue pages.
  • Actually the path is not needed here.
Tue, Nov 14, 11:47Config, UX
waldyrious updated the diff for D25466: Improve command line prompts in setup issue pages.
  • Fix missing arguments to hsprintf; re-add csprintf to string using the %R conversion
Tue, Nov 14, 11:46Config, UX
waldyrious updated the diff for D25466: Improve command line prompts in setup issue pages.
  • Improve prompts: use hsprintf instead of cprintf, add missing <tt> markers, simplify syntax
Tue, Nov 14, 11:40Config, UX
waldyrious attached a referenced file: F397870: image.png.
Tue, Nov 14, 11:36Config, UX
waldyrious added a comment to D25466: Improve command line prompts in setup issue pages.

For future reference, I went digging for the difference between csprintf() and hsprintf(), and found the following:

Tue, Nov 14, 11:36Config, UX
valerio.bozzolan edited the content of Next Up.
Tue, Nov 14, 11:30
aklapper added a comment to T15547: Alternative hashtag of milestone has no primary slug, thus project URL redirects to 404 /tag//.

This might be a duplicate of T15636

Tue, Nov 14, 11:20
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

As an aside, what do you think of https://github.com/campbsb/example-phorge-php-project - should we put a selection of example projects on https://we.phorge.it, either as individual repos, or one repo containing a bunch of examples, or somewhere in the arcanist repo ?

Tue, Nov 14, 11:17Arcanist
valerio.bozzolan added a comment to D25418: Catch RuntimeException: mb_convert_encoding(): Illegal character encoding specified at PhabricatorTextDocumentEngine.php:73.

(I will immediately hard-approve this lovely change as soon as we share whatever ACK like "Yeah let's log the planet" or "Yeah let's just show a user message")

Tue, Nov 14, 11:14
valerio.bozzolan added a comment to D25418: Catch RuntimeException: mb_convert_encoding(): Illegal character encoding specified at PhabricatorTextDocumentEngine.php:73.

OK the situation is a bit more clear now.

Tue, Nov 14, 11:12
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

More complete test case added to D25472 which demonstrates the following error:

Tue, Nov 14, 11:10Arcanist
Sten updated the test plan for D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Tue, Nov 14, 11:06
valerio.bozzolan added a comment to T15624: RuntimeException: mb_convert_encoding(): Illegal character encoding specified at PhabricatorTextDocumentEngine.php:73.

Interesting. I think this specific crash was not caused by a specific document, but it was probably caused by a specific nonsense request with a nonsense encoding.

Tue, Nov 14, 10:56
aklapper updated the diff for D25418: Catch RuntimeException: mb_convert_encoding(): Illegal character encoding specified at PhabricatorTextDocumentEngine.php:73.

Good points. This patch puts also the exception message into phlog.

Tue, Nov 14, 10:34
Sten updated the test plan for D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Tue, Nov 14, 09:57
Sten requested review of D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Tue, Nov 14, 09:57
Sten added a revision to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option: D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Tue, Nov 14, 09:57Arcanist
valerio.bozzolan added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.
In T15667#14211, @Sten wrote:

how many people use Phabricator/Phorge :-(

Tue, Nov 14, 09:38Arcanist
valerio.bozzolan edited the content of Organizations Using Phorge.
Tue, Nov 14, 09:37
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

I guess the documentation needs an update on page https://we.phorge.it/book/phorge/article/arcanist_lint_unit/ and/or https://we.phorge.it/book/phorge/article/arcanist_new_project/

Tue, Nov 14, 09:28Arcanist
valerio.bozzolan accepted D25276: Add support for secure connections to the database.

I basically completely trust speck if they say this works with TLS, and I verified this works without TLS. Tried with / without this configuration.

Tue, Nov 14, 09:28
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

6 year old in-your-face unreported bugs like these make me wonder how many people use Phabricator/Phorge :-(

Tue, Nov 14, 09:22Arcanist
valerio.bozzolan added a comment to D25418: Catch RuntimeException: mb_convert_encoding(): Illegal character encoding specified at PhabricatorTextDocumentEngine.php:73.

(I share the same question from Sten)

Tue, Nov 14, 08:27
valerio.bozzolan accepted D25422: Fix a PHP 8.1 deprecated use of strlen with a NULL argument on commit page.

I'm 5% scared by this since in 8 months we may discover that $author could be an object and so an user may report our well-known exception in some cases. But, the method name is getAuthorString() so I'm quite sure that Evan Priestley already choosen that name for a reason, and so, probably the getAuthor() is always a string or null and nothing else, so it's nice to report alien things with an exception, as proposed here.

Tue, Nov 14, 08:20
valerio.bozzolan triaged T15668: [Performance] Avoid to calculate a string length, if we are just interested in "is empty" as Wishlist priority.
Tue, Nov 14, 08:09
valerio.bozzolan created T15668: [Performance] Avoid to calculate a string length, if we are just interested in "is empty".
Tue, Nov 14, 08:00
Sten claimed T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.
Tue, Nov 14, 07:40Arcanist
Sten created T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.
Tue, Nov 14, 07:40Arcanist
valerio.bozzolan added a comment to D25428: Fix the issue about persistent chat setting icon being clickable when the chat is collapsed.
  • We don't use document.querySelector - we use JX.DOM.find, you can see an example in line 43
Tue, Nov 14, 07:38
valerio.bozzolan added a comment to D25466: Improve command line prompts in setup issue pages.

I love your change. Simple and effective and without impact on other parts.

Tue, Nov 14, 07:29Config, UX
valerio.bozzolan updated the name of F397638: Prompt proposal.png from "Screenshot_2023_11_14_075710.png" to "Prompt proposal.png".
Tue, Nov 14, 06:57

Mon, Nov 13

waldyrious updated the diff for D25466: Improve command line prompts in setup issue pages.
  • Fix double prompt marker
Mon, Nov 13, 23:30Config, UX
waldyrious added inline comments to D25466: Improve command line prompts in setup issue pages.
Mon, Nov 13, 23:28Config, UX
waldyrious added a comment to D25466: Improve command line prompts in setup issue pages.

Nice catches, @valerio.bozzolan 鈥 will fix.

Mon, Nov 13, 23:23Config, UX
valerio.bozzolan updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

celerity

Mon, Nov 13, 21:37
valerio.bozzolan updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Hoping to be useful, proposing minor things. Feel free to say "AAAH NOPE" :D

Mon, Nov 13, 21:36
valerio.bozzolan added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
NOTE: In short, feel free to just see the last diff and ignore this comment. If you like, nice! <3 If you don't, say so and I rollback. Thanks :D
Mon, Nov 13, 21:30
valerio.bozzolan accepted D25471: Updates for Mercurial's HTTP protocol.

I completely trust Speck and Evan Priestley that already reviewed this in secure dot. So...

Mon, Nov 13, 21:02
valerio.bozzolan edited the content of Code Differences between Phabricator and Phorge.
Mon, Nov 13, 21:00
valerio.bozzolan foisted D25471: Updates for Mercurial's HTTP protocol upon speck.

Restoring original author :) Thanks speck

Mon, Nov 13, 20:59
valerio.bozzolan requested review of D25471: Updates for Mercurial's HTTP protocol.
Mon, Nov 13, 20:58
valerio.bozzolan edited the content of Update From Phabricator.
Mon, Nov 13, 20:41
valerio.bozzolan updated subscribers of Update From Phabricator.

Hi @Cigaryno thanks for this change but we are causing some confusion since the mentioned commit 40b272fa is not in Phorge (as reported by the kind @jmeador from Mozilla). I will try editing a bit. Feel free to review then.

Mon, Nov 13, 20:32
valerio.bozzolan edited the content of Code Differences between Phabricator and Phorge.
Mon, Nov 13, 19:35
valerio.bozzolan edited the content of Code Differences between Phabricator and Phorge.
Mon, Nov 13, 18:48
valerio.bozzolan retitled D25441: Fix possible array to string conversion renaming Pholio Mockup image from Fix array to string conversion renaming several Pholio mockup images at once
Mon, Nov 13, 17:27
valerio.bozzolan accepted D25441: Fix possible array to string conversion renaming Pholio Mockup image.

So I'm 99.99% sure that $old and $new always are PHID=>title array and always with count($that) === 1 - whenever you rename a single photo or multiple ones. So, I see this as totally OK.

Mon, Nov 13, 17:23
valerio.bozzolan added a comment to T15646: Renaming several Pholio Mockup images creates neverending daemon task (PhutilProxyException due to Array to string conversion).

Interestingly I was able to reproduce the issue with this nonsense patch, in the hope to emulate an email plain-text generation, even if it's not the case:

Mon, Nov 13, 17:12Affects-Wikimedia, Bug Reports
valerio.bozzolan added a comment to T15646: Renaming several Pholio Mockup images creates neverending daemon task (PhutilProxyException due to Array to string conversion).

For interesting reasons, I cannot, in any way, get an email about somebody renaming a single Mockup photo. And, my email preferences are like "email me for everything from every application" and email delivery is "email me about myself".

Mon, Nov 13, 17:10Affects-Wikimedia, Bug Reports
valerio.bozzolan added a comment to D25441: Fix possible array to string conversion renaming Pholio Mockup image.

I'm now quite sure that $old and $new always are array, and each one consists in a single element with key PHID and value => title.

Mon, Nov 13, 16:52
valerio.bozzolan added a comment to Q83: Pull security fixes from Mozilla's fork.

(I also cannot see T15665)

Mon, Nov 13, 15:06Security, Phorge
20after4 added a comment to T15642: Feed Transaction Logs: Exception: Query overheated: examined more than 1,010 raw rows without finding 101 visible objects..

Maybe this is a clue?

Mon, Nov 13, 14:58
valerio.bozzolan added a comment to T15486: Do not expose "Contact Numbers" in user settings when no SMS support is set up in Phorge.

Also, I'm OK with the change also because of GDPR's principle of minimization. I mean, Phorge avoids to collect unnecessary data, and this is nice.

Mon, Nov 13, 14:52Policy
valerio.bozzolan edited the content of Next Up.
Mon, Nov 13, 14:51
valerio.bozzolan added a comment to T15486: Do not expose "Contact Numbers" in user settings when no SMS support is set up in Phorge.

As a side note, it's possible that somebody in the world was using the Phone number feature in a way that was then integrated with their custom management system, accessing this information via plain SQL.

Mon, Nov 13, 14:37Policy
valerio.bozzolan added a comment to D25363: Match yourself from Imported Events Invitees.

I've adopted PhabricatorPeopleUserEmailQuery as suggested.

Mon, Nov 13, 13:37
valerio.bozzolan updated the diff for D25363: Match yourself from Imported Events Invitees.

Apply review tips

Mon, Nov 13, 13:33
aklapper closed D25452: Do not expose Contact Numbers settings panel when no SMS support configured.
Mon, Nov 13, 13:04
aklapper closed T15486: Do not expose "Contact Numbers" in user settings when no SMS support is set up in Phorge as Resolved by committing rP282e37aaf682: Do not expose Contact Numbers settings panel when no SMS support configured.
Mon, Nov 13, 13:04Policy
aklapper committed rP282e37aaf682: Do not expose Contact Numbers settings panel when no SMS support configured.
Do not expose Contact Numbers settings panel when no SMS support configured
Mon, Nov 13, 13:04
valerio.bozzolan edited the content of Next Up.
Mon, Nov 13, 11:09
valerio.bozzolan added a comment to D25421: Audit Feed: less verbose when the author is the committer.

(Hide all inline comments or this is unreadable)

Mon, Nov 13, 10:28
valerio.bozzolan updated the diff for D25421: Audit Feed: less verbose when the author is the committer.

Small improvement thanks to review tip

Mon, Nov 13, 10:26
valerio.bozzolan added a comment to D25467: Align logo image and text in site header.
In D25467#13458, @l2dy wrote:

I have verified the fix on Firefox. Are you sure phabricator/c0bdb5b4/core.pkg.css is loaded?

Mon, Nov 13, 10:07
l2dy closed D25467: Align logo image and text in site header.
Mon, Nov 13, 09:56
l2dy committed rPaa8af1d79e8b: Align logo image and text in site header.
Align logo image and text in site header
Mon, Nov 13, 09:56
l2dy added a comment to D25467: Align logo image and text in site header.

Uh! Nice!

Interestingly this CSS change only has effects on Chromium 119 and it does not change anything in my Mozilla Firefox 118. Can you give a quick look?

Mon, Nov 13, 09:46
valerio.bozzolan added inline comments to D25466: Improve command line prompts in setup issue pages.
Mon, Nov 13, 08:21Config, UX
valerio.bozzolan added inline comments to D25466: Improve command line prompts in setup issue pages.
Mon, Nov 13, 08:19Config, UX
valerio.bozzolan accepted D25467: Align logo image and text in site header.

Interestingly this CSS change only has effects on Chromium 119 and it does not change anything in my Mozilla Firefox 118. Can you give a quick look?

Mon, Nov 13, 07:56