Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading through and updating README.md (attempt 2) #20

Merged
merged 19 commits into from
Apr 9, 2023

Conversation

Mikaela
Copy link
Contributor

@Mikaela Mikaela commented Mar 13, 2023

This is revival/rebase of #18 where I missed replies earlier (or have no memory of them).

Resolves: #17
Resolves: #19

TODO

  • I need to figure out what is doctoc and where to get it on Fedora
  • I mostly replaced freenode with OFTC as I understand that the alive projects went there, but is that correct?

Mikaela

This comment was marked as resolved.

@Mikaela Mikaela marked this pull request as ready for review March 13, 2023 09:38
@Mikaela Mikaela requested review from frzifus and Thaodan March 13, 2023 09:38
Copy link
Member

@zarelit zarelit left a comment

Choose a reason for hiding this comment

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

Not a SFOS user anymore but the changes looks okay to me

rev: v2.2.0
hooks:
- id: doctoc
args: [--update-only]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes doctoc not force itself on everything, while it can be enabled by adding the following anywhere in a file:

<!-- START doctoc -->
<!-- END doctoc -->

However it looks like all files were doctoced before I enabled it, and I don't know whether non-doctoced are desirable.

@@ -32,3 +33,12 @@ edit your PR before we merge it. There's no need to open a new PR, just edit
the existing one. If you're not sure how to do that,
[here is a guide](https://github.com/RichardLitt/knowledge/blob/master/github/amending-a-commit-guide.md)
on the different ways you can update your PR so that we can merge it.

## pre-commit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a feeling I cannot English here.

@Mikaela Mikaela requested a review from zarelit March 25, 2023 18:17
Copy link
Contributor Author

@Mikaela Mikaela left a comment

Choose a reason for hiding this comment

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

I may be overthinking this

* text=auto
README.md merge=union
* text=auto eol=lf merge=union
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't any complicated programming language code here, so I think merge=union should be a safe strategy for all files.

eol=lf workarounds thlorenz/doctoc#161 in case anyone uses Windows.

Comment on lines +1 to +5
root = true

[*]
end_of_line = lf
charset = utf-8
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://editorconfig.org/ configures settings that will apply to multiple text editors just for this repository.

  • root=true means that .editorconfig capable text-editors (see link earlier) won't go looking for configuration from top leve l directories.
  • line endings are enforced to be lf, which the editors should detect automatically though, as a pair to .gitattributes eol=lf and to workaround Unusable as 'pre-commit' hook, since it will always fail. thlorenz/doctoc#161 just in case anyone uses Windows.
  • well I certainly hope everyone uses utf-8

@Mikaela
Copy link
Contributor Author

Mikaela commented Apr 2, 2023

It has been a week from previous comments, is this OK to merge?

@Mikaela Mikaela merged commit 5112a90 into sailfishos-community:master Apr 9, 2023
@Mikaela Mikaela deleted the mikaela-readthrough branch April 9, 2023 04:47
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.

Arabic Community Switch away from freenode
3 participants