Skip to content

Packages submodule, git files, and docs improvements#279

Merged
trishume merged 7 commits into
trishume:masterfrom
jrappen:master
Jan 31, 2020
Merged

Packages submodule, git files, and docs improvements#279
trishume merged 7 commits into
trishume:masterfrom
jrappen:master

Conversation

@jrappen

@jrappen jrappen commented Jan 28, 2020

Copy link
Copy Markdown
Contributor

Update changelog:

  • added changelog info from releases to changelog file

Changes to Packages submodule target:

@trishume trishume left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I like these changes, except for the CSS colors.

The CSS colors are clearly generated but the generation code isn't included and it's just pasted into a file that has other non-generated code. Also if the CSS colors are going to be used they'll need to be in some kind of map indexed by strings and not just Rust constants. I'd prefer you just remove the CSS color stuff from this PR, and if you plan on implementing sublime-color-scheme support you can include it there and ideally in a separate file containing only generated code and checking in the script that generated that file. Or use a different approach like the default syntax packs use where you load a binary dump of a string to color map that's generated by a program under examples.

Otherwise I like these changes, especially the much improved changelog and the new .gitattributes. If you remove the CSS color stuff I'm happy to merge this.

@trishume

Copy link
Copy Markdown
Owner

Oh also I think in order to get the Packages actually updated you have to do some check out step in the submodule. I forget exactly how submodules work but I'm pretty sure this PR doesn't actually update the packages, just makes future updates target the correct branch. Also when really updating the packages you need to rebuild the checked in binary dumps.

Not updating the packages in this PR is fine though. I'm pretty sure based on the other issues that updating the packages might not work.

reverting changes from 3015827 based on comment in #279
@jrappen

jrappen commented Jan 30, 2020

Copy link
Copy Markdown
Contributor Author

Can't test the build on this machine. Might try later today.

Pushed the requested changes regarding css names for now.

@jrappen jrappen changed the title Few minor improvements Packages submodule, git files, and docs improvements Jan 30, 2020
@trishume trishume merged commit 17b3a95 into trishume:master Jan 31, 2020
@jrappen jrappen mentioned this pull request Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants