Paths

Table of Contentst

Differential D14236

Initial inclusion of Helix-Editor
ClosedPublic
Actions

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

Tags

None

Referenced Files

	F11006276: D14236.id34729.diff
	Tue, Aug 1, 2:34 PM

	F11006274: D14236.id34728.diff
	Tue, Aug 1, 2:34 PM

	F11006273: D14236.id34717.diff
	Tue, Aug 1, 2:34 PM

	F11006272: D14236.id34716.diff
	Tue, Aug 1, 2:34 PM

	F11006271: D14236.id.diff
	Tue, Aug 1, 2:34 PM

	F11003274: D14236.diff
	Mon, Jul 31, 12:46 PM

	F10977031: D14236.diff
	Sat, Jul 22, 1:37 PM

	F10967806: D14236.diff
	Thu, Jul 20, 12:20 PM

View All 9 Files

Subscribers

Details

Reviewers

Group Reviewers

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

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

maveonair created this revision.Tue, Jul 11, 6:31 AM

Herald added a reviewer: Triage Team. · View Herald TranscriptTue, Jul 11, 6:31 AM

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

Harbormaster completed remote builds in B5258: Diff 34716.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
7	Use the strings from here: https://spdx.org/licenses/, so this only needs to be `MPL-2.0`
7	Please add a `homepage` key under license
16	This is part of `system.devel` so it can be removed
19	What is this doing? I was able to build with it
23	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
27	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

davidjharder added inline comments.Tue, Jul 11, 1:58 PM

package.yml
19	sorry, meant "without it"

EbonJaeger added a subscriber: EbonJaeger.Tue, Jul 11, 2:03 PM

joebonrichie added a subscriber: joebonrichie.Tue, Jul 11, 2:04 PM

joebonrichie added inline comments.

package.yml
26	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

Harbormaster completed remote builds in B5259: Diff 34717.Tue, Jul 11, 2:11 PM

maveonair added inline comments.Tue, Jul 11, 2:12 PM

package.yml
19	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".

Throw in your maintainer file and this should be good to go!

Add maintainer file

Harbormaster completed remote builds in B5270: Diff 34728.Wed, Jul 12, 11:42 AM

Looks good. Thanks

This revision is now accepted and ready to land.Wed, Jul 12, 1:36 PM

Closed by commit R5766:f388c9c6242d: Initial inclusion of Helix-Editor (authored by maveonair, committed by davidjharder). · Explain WhyWed, Jul 12, 1:43 PM

This revision was automatically updated to reflect the committed changes.

davidjharder added a commit: R5766:f388c9c6242d: Initial inclusion of Helix-Editor.

Revision Contents
Changeset List

Path

Size

4 lines

1 line

148 lines

694 lines

5 lines

abi_used_symbols

204 lines

files/

3 lines

27 lines

pspec_x86_64.xml

713 lines

Diff 34729

MAINTAINERS.md

Loading...

Makefile

Loading...

abi_libs

Loading...

abi_symbols

Loading...

abi_used_libs

Loading...

abi_used_symbols

Loading...

files/hx

Loading...

package.yml

Loading...

pspec_x86_64.xml

Loading...