Introducing DEVELOPMENT.md #5209

zhitkoff · 2023-08-26T20:17:55Z

Adding DEVELOPMENT.md with more detailed instruction on setting local development environment

tertsdiepraam · 2023-08-26T21:14:54Z

I think you might have accidentally included some commits from another branch. Could you remove those?

tertsdiepraam

Cool! I think as general feedback I want this to be a bit more concise where possible or at least structure it in a way where people can get started quickly.

Maybe we could also use tricks like the HTML <details> element to hide platform-specific instructions?

I'll also remove the link to #5195 because this won't be enough to close that issue.

tertsdiepraam · 2023-08-26T21:56:06Z

DEVELOPMENT.md

+### GNU utils and prerequisites
+If you are developing on Linux, most likely you already have all/most GNU utilities and prerequisites installed. 
+To make sure, please check GNU coreutils [README-prereq](https://github.com/coreutils/coreutils/blob/master/README-prereq)
+
+**Tip:On MacOS** you will need to install [Homebrew](https://docs.brew.sh/Installation) and use it to install the following formulas:


I think this should be moved down because it's only important when running the GNU test suite (unless there are also some things you need for regular development).

There are a few things that are needed outside of GNU tests, namely pre-commit (even though it is currently broken on MacOS). Also Homebrew seems to be inevitable for development environment on MacOS anyway, and especially so for this project

But tools like pre-commit also have their own section with installation instructions already. For some others I don't understand what they are used for (like wget & texinfo are those used by GNU?). So maybe we could also add some comments about which packages are used for what.

I do like providing an easy list like this, but maybe that could go at the end and you can link to it here? I think it's quite important that the start of the document is relevant for everyone. What do you think of a structure like this?

## Before you start ## Tools [All tools and cross-platform stuff here] ## Tips for installing on Mac ## Tips for installing on Windows

I appreciate you dedication to helping newcomers btw. It's also good to have someone with a mac perspective around. I had no idea about all the extra stuff that was needed 😄

makes sense. and yes, most of the Homebrew formulas are needed by GNU build/tests - i just ran it over and over, fixing failures one by one and that is the final list.
Will update with structure you suggested

tertsdiepraam · 2023-08-26T22:01:10Z

DEVELOPMENT.md

+**Tip:On MacOS** you will need to install C compiler & linker:
+`xcode-select --install`
+
+**Tip:On Windows** On Windows you'll need the MSVC build tools for Visual Studio 2013 or later.
+To acquire the build tools, you’ll need to install Visual Studio 2022. When asked which workloads to install, include:
+- Desktop Development with C++
+- The Windows 10 or 11 SDK
+- The English language pack component, along with any other language pack of your choosing


Do we need to mention all of this? If it's a very common pitfall for newcomers I'm all for including it, but it feels maybe a bit too detailed? All bits of information here also need to be kept up to date (i.e. when people should install Visual Studio 2023 instead of 2022). So, just saying "install rust via rustup" should probably be fine. We could also link to more extensive installation guides instead.

This is how simple it could be: https://github.com/nushell/nushell/blob/main/CONTRIBUTING.md#setup (though we could add a bit more info if we really want to).

True, could be simplified. On the Nu shell example though - they offload all details to linked https://www.nushell.sh/book/installation.html for installing Rust toolchain and all dependencies/prerequisites - that makes their CONTRIBUTING.md clean and simple. Woulkd the equivalent in our case be moving all those details out of CONTRIBUTING.md to DEVELOPMENT.md ?

Hmm, yeah I guess you're right. We can keep it and just trim it a little bit.

tertsdiepraam · 2023-08-26T22:03:23Z

DEVELOPMENT.md

+
+## Code coverage
+
+<!-- spell-checker:ignore (flags) Ccodegen Coverflow Cpanic Zinstrument Zpanic -->


Let's move this to the top.

to the top of the Testing section or somewhere else?

tertsdiepraam · 2023-08-26T22:07:46Z

DEVELOPMENT.md

+
+## Before you start
+
+For this guide we assume that you already have GitHub account and have `git` and your favorite code editor or IDE installed and configured to work with GitHub.


configured to work with GitHub.

Nit, but the IDE doesn't need GitHub integration.

tertsdiepraam · 2023-08-26T22:09:22Z

DEVELOPMENT.md

+For this guide we assume that you already have GitHub account and have `git` and your favorite code editor or IDE installed and configured to work with GitHub.
+Before you start working on coreutils, please follow these steps:
+
+1. Fork [coreutils repository](https://github.com/uutils/coreutils) to your GitHub account [ref:](https://docs.github.com/en/get-started/quickstart/fork-a-repo)


Suggested change

1. Fork [coreutils repository](https://github.com/uutils/coreutils) to your GitHub account [ref:](https://docs.github.com/en/get-started/quickstart/fork-a-repo)

1. Fork the [coreutils repository](https://github.com/uutils/coreutils) to your GitHub account [ref:](https://docs.github.com/en/get-started/quickstart/fork-a-repo)

I'd also like to have some more text as the link than "ref". Maybe See [this GitHub guide](...) for more information.

github-actions · 2023-08-28T06:18:52Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail-2/inotify-dir-recreate

zhitkoff · 2023-09-01T17:45:39Z

made changes to DEVELOPMENT.md and simplified CONTRIBUTING.md (offload and link all dev details to DEVELOPMENT.md). There are some TODOs left - looking for feedback

sylvestre · 2023-09-02T14:34:27Z

DEVELOPMENT.md

@@ -0,0 +1,331 @@
+<!-- spell-checker:ignore reimplementing toybox RUNTEST CARGOFLAGS nextest prereq autopoint gettext texinfo automake findutils shellenv libexec gnubin -->


Suggested change

Thanks! If you do not mind me asking - what are you using for spell checker? The VSCode spellchecker extension did not catch this, even though it is enabled for markdown files by default

Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>

sylvestre · 2023-09-07T14:33:09Z

please let me know when it is ready to be reviewed :)

zhitkoff · 2023-09-07T15:10:39Z

please let me know when it is ready to be reviewed :)

yep, should be ready

github-actions · 2023-09-07T15:27:47Z

GNU testsuite comparison:

GNU test failed: tests/rm/rm2. tests/rm/rm2 is passing on 'main'. Maybe you have to rebase?

sylvestre · 2023-09-07T15:50:14Z

you know that merging isn't necessary ? ;)
(esp for doc)

sylvestre · 2023-09-07T15:57:41Z

CONTRIBUTING.md

-<!-- spell-checker:ignore (flags) Ccodegen Coverflow Cpanic Zinstrument Zpanic -->
-
-Code coverage report can be generated using [grcov](https://github.com/mozilla/grcov).
+## TODO : Any guidelines for branching, PRs, etc?


please remove it, we don't do it yet

apologies - have been traveling. will do

sylvestre · 2023-09-07T15:58:02Z

CONTRIBUTING.md


-If you are using stable version of Rust that doesn't enable code coverage instrumentation by default
-then add `-Z-Zinstrument-coverage` flag to `RUSTFLAGS` env variable specified above.
+***TODO*** The technical details on how to generate report locally are offloaded to DEVELOPMENT.md


please fix this one too

zhitkoff · 2023-09-23T21:46:48Z

you know that merging isn't necessary ? ;)
(esp for doc)

thanks, will keep that in mind in the future.

tertsdiepraam

This is looking very good! The content is great, I just have few comments on the formatting.

DEVELOPMENT.md

tertsdiepraam · 2023-09-24T10:33:32Z

DEVELOPMENT.md

+
+If you're using rustup to install and manage your Rust toolchains, `clippy` and `rustfmt` are usually already installed. If you are using one of the alternative methods, please make sure to install them manually. See following sub-sections for their usage: [clippy](#clippy) [rustfmt](#rustfmt)
+
+***Tip*** You might also need to add 'llvm-tools':


Suggested change

***Tip*** You might also need to add 'llvm-tools':

***Tip***: You might also need to add 'llvm-tools':

It might also be nice to mention what those tools are for. Is it code cov?

yep, will update

tertsdiepraam · 2023-09-24T10:34:47Z

DEVELOPMENT.md

+Before you start working on coreutils, please follow these steps:
+
+1. Fork the [coreutils repository](https://github.com/uutils/coreutils) to your GitHub account.
+***Tip:*** See [this GitHub guide](https://docs.github.com/en/get-started/quickstart/fork-a-repo) for more information on this step


Very nitpicky, but there's a few periods missing at the end of sentences throughout this doc.

DEVELOPMENT.md

Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>

sylvestre · 2023-10-02T15:43:27Z

DEVELOPMENT.md

+On MacOS you will need to install [Homebrew](https://docs.brew.sh/Installation) and use it to install the following Homebrew formulas:
+
+```shell
+brew install coreutils


it cannot be done on a single line ?

it sure can, but how about this version instead?

brew install \ coreutils \ autoconf \ gettext \ wget \ texinfo \ xz \ automake \ gnu-sed \ m4 \ bison \ pre-commit \ findutils

tertsdiepraam

I'm happy with this! I'll let Sylvestre make the final merge, but it's got my approval. Thanks!

Adding DEVELOPMENT.md

c48cc94

zhitkoff mentioned this pull request Aug 26, 2023

Improving CONTRIBUTING.md #5195

Closed

tertsdiepraam reviewed Aug 26, 2023

View reviewed changes

zhitkoff and others added 4 commits August 27, 2023 14:07

Delete build-gnu.sh

150817f

should not be modified for issue5195

52cf6dc

updates based on feedback

6f470a1

Improving DEVELOPMENT.md

7005397

refactoring CONTRIBUTING.md with changes to DEVELOPMENT.md

d819e2d

sylvestre reviewed Sep 2, 2023

View reviewed changes

zhitkoff and others added 2 commits September 2, 2023 14:44

Update DEVELOPMENT.md

7588624

Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>

Merge branch 'main' into zhitkoff/issue5195

71dbb64

zhitkoff marked this pull request as ready for review September 7, 2023 15:09

Merge branch 'main' into zhitkoff/issue5195

0d70b54

sylvestre reviewed Sep 7, 2023

View reviewed changes

zhitkoff and others added 2 commits September 23, 2023 17:38

edits based on review comments

c7db725

Merge branch 'main' into zhitkoff/issue5195

8938b45

tertsdiepraam reviewed Sep 24, 2023

View reviewed changes

sylvestre and others added 2 commits September 25, 2023 13:33

improve wording

25d6350

Co-authored-by: Terts Diepraam <terts.diepraam@gmail.com>

edits per review comments

37e8c1f

zhitkoff requested review from sylvestre and tertsdiepraam October 2, 2023 15:36

sylvestre reviewed Oct 2, 2023

View reviewed changes

development.md: formatting

b2860a3

tertsdiepraam approved these changes Oct 2, 2023

View reviewed changes

sylvestre merged commit 9f6a720 into uutils:main Oct 2, 2023
45 checks passed

zhitkoff deleted the zhitkoff/issue5195 branch November 18, 2023 16:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing DEVELOPMENT.md #5209

Introducing DEVELOPMENT.md #5209

zhitkoff commented Aug 26, 2023 •

edited

Loading

tertsdiepraam commented Aug 26, 2023

tertsdiepraam left a comment •

edited

Loading

tertsdiepraam Aug 26, 2023

zhitkoff Aug 28, 2023 •

edited

Loading

tertsdiepraam Aug 28, 2023 •

edited

Loading

zhitkoff Aug 29, 2023

tertsdiepraam Aug 26, 2023

zhitkoff Aug 27, 2023 •

edited

Loading

tertsdiepraam Aug 27, 2023

tertsdiepraam Aug 26, 2023

zhitkoff Aug 28, 2023

tertsdiepraam Aug 26, 2023

tertsdiepraam Aug 26, 2023

github-actions bot commented Aug 28, 2023

zhitkoff commented Sep 1, 2023

sylvestre Sep 2, 2023

zhitkoff Sep 2, 2023

sylvestre commented Sep 7, 2023

zhitkoff commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

sylvestre commented Sep 7, 2023

sylvestre Sep 7, 2023

sylvestre Sep 23, 2023

zhitkoff Sep 23, 2023

sylvestre Sep 7, 2023

zhitkoff commented Sep 23, 2023

tertsdiepraam left a comment

tertsdiepraam Sep 24, 2023 •

edited

Loading

zhitkoff Sep 25, 2023

tertsdiepraam Sep 24, 2023

sylvestre Oct 2, 2023

zhitkoff Oct 2, 2023

sylvestre Oct 2, 2023

tertsdiepraam left a comment


		## Code coverage

		<!-- spell-checker:ignore (flags) Ccodegen Coverflow Cpanic Zinstrument Zpanic -->


		## Before you start

		For this guide we assume that you already have GitHub account and have `git` and your favorite code editor or IDE installed and configured to work with GitHub.

	1. Fork [coreutils repository](https://github.com/uutils/coreutils) to your GitHub account [ref:](https://docs.github.com/en/get-started/quickstart/fork-a-repo)
	1. Fork the [coreutils repository](https://github.com/uutils/coreutils) to your GitHub account [ref:](https://docs.github.com/en/get-started/quickstart/fork-a-repo)

		@@ -0,0 +1,331 @@
		<!-- spell-checker:ignore reimplementing toybox RUNTEST CARGOFLAGS nextest prereq autopoint gettext texinfo automake findutils shellenv libexec gnubin -->


		If you're using rustup to install and manage your Rust toolchains, `clippy` and `rustfmt` are usually already installed. If you are using one of the alternative methods, please make sure to install them manually. See following sub-sections for their usage: [clippy](#clippy) [rustfmt](#rustfmt)

		*Tip* You might also need to add 'llvm-tools':

	*Tip* You might also need to add 'llvm-tools':
	*Tip*: You might also need to add 'llvm-tools':

Introducing DEVELOPMENT.md #5209

Introducing DEVELOPMENT.md #5209

Conversation

zhitkoff commented Aug 26, 2023 • edited Loading

tertsdiepraam commented Aug 26, 2023

tertsdiepraam left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhitkoff Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

tertsdiepraam Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhitkoff Aug 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 28, 2023

zhitkoff commented Sep 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sylvestre commented Sep 7, 2023

zhitkoff commented Sep 7, 2023

github-actions bot commented Sep 7, 2023

sylvestre commented Sep 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhitkoff commented Sep 23, 2023

tertsdiepraam left a comment

Choose a reason for hiding this comment

tertsdiepraam Sep 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tertsdiepraam left a comment

Choose a reason for hiding this comment

zhitkoff commented Aug 26, 2023 •

edited

Loading

tertsdiepraam left a comment •

edited

Loading

zhitkoff Aug 28, 2023 •

edited

Loading

tertsdiepraam Aug 28, 2023 •

edited

Loading

zhitkoff Aug 27, 2023 •

edited

Loading

tertsdiepraam Sep 24, 2023 •

edited

Loading