Initial inclusion of Helix-Editor
ClosedPublic
Actions

Authored by maveonair on Tue, Jul 11, 6:31 AM.

Details

Reviewers

davidjharder

Group Reviewers

Triage Team

Commits

R5766:f388c9c6242d: Initial inclusion of Helix-Editor

Summary

Initial inclusion of Helix-Editor

Fixes: T10575

Test Plan

Run it against differnet file types to validate that syntax higlighting works correctly

Branch

main

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 5258
Build 5258: arc lint + arc unit

maveonair requested review of this revision.Tue, Jul 11, 6:31 AM

This is looking pretty good.

I have a pile of nits :

The diff title should be: Initial inclusion of helix
To reference the related task add the text Fixes Txxxxx into the summary somewhere
See inline comments in package.yml

package.yml
6	Use the strings from here: https://spdx.org/licenses/, so this only needs to be `MPL-2.0`
6	Please add a `homepage` key under license
15	This is part of `system.devel` so it can be removed
18	What is this doing? I was able to build with it
22	General comments: You do not need to enclose all these paths in quotes We remove the space in `-Dm 0xxx` as a matter of taste
26	The license is specified higher up, this can be removed

davidjharder requested changes to this revision.Tue, Jul 11, 1:52 PM

This revision now requires changes to proceed.Tue, Jul 11, 1:52 PM

One more thing: please add your MAINTAINER file

package.yml
18	sorry, meant "without it"

joebonrichie added inline comments.

package.yml
25	octal permissions e.g. 00644

maveonair retitled this revision from Add Helix 23.05 for inclusion T10575 to Initial inclusion of Helix-Editor.Tue, Jul 11, 2:04 PM

maveonair edited the summary of this revision. (Show Details)

Code review changes

package.yml
18	I can't build it without because Helix tries to check the grammar sources with Git during the build and therefore fails with "detected dubious ownership in repository".