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

Root directory content refactoring #1725

Merged
merged 12 commits into from
Jul 2, 2023
Merged

Root directory content refactoring #1725

merged 12 commits into from
Jul 2, 2023

Conversation

ia
Copy link
Collaborator

@ia ia commented Jun 27, 2023

  • Please check if the PR fulfills these requirements
  • The changes have been tested locally
  • There are no breaking changes
  • What kind of change does this PR introduce?
    Rearrangement of the project root directory.

  • What is the current behavior?
    When a person who is curious about the project, downloads and/or opens it on github, there are a lot of stockpiled files in a root directory for different purposes/automation/deploy/documentation tasks.

  • What is the new behavior (if this is a feature change)?
    Since root directory is a store front of a project, such amazing project as this one deserves a bit more tidy main entrance in my humble opinion.

Main locations/files modifications:

  • move text from Bootup Logo/README.md to README.md
  • replace build.sh & start_dev.sh by Makefile
  • move community script to flash TS100 from Flashing one file directory to newly created Scripts directory
  • move Dockerfile to Scripts/IronOS.Dockerfile
  • move ci directory, mkdocs.yml, LICENSE_RELEASE, and PULL_REQUEST_TEMPLATE inside Scripts directory
  • update & modify related configuration files according to changes
  • root Makefile can run docker-related commands, generate docs and pass-through targets for source/ directory

Partially it's like putting under the rug files which must be with the project, but they don't have to be in direct sight.

  • Other information:
    What have been tested successfully:
  • local run of make docker-shell
  • local run of make docker-build
  • local run of make docs
  • github actions (according to logs from github, changes in .github/workflow have been applied) with generation of artifacts successfully

What haven't been tested:

  • generation & deploy of documentation using make docs for RtD online location (not sure how better to test it - to make pull request around docs branch?)
  • workflows on hosts of other developers - some of your local scripts may be terribly broken after this patch :(

Probably section about Bootup Logo can be safely removed from README.md completely since there is mention of Bootup Logo feature in a feature list with link to official documentation.

As always, let me know what you think. Thanks for any feedback.

…tate README to main README; - replace separate root scripts build.sh and start_dev.sh by root Makefile; - make Scripts directory and move there: flash_ts100_linux.sh script, ci/ directory, dockerfile, LICENSE_RELEASE, and PULL_REQUEST_TEMPLATE; - reconfigure build & deploy scripts according to changes
@ia
Copy link
Collaborator Author

ia commented Jun 27, 2023

Oh, and docker-compose is IronOS.yml now - it can't be inside of Scripts directory due to how docker-compose handles volumes/mappings/COPY : access to source dir is required to build firmware obviously, but with both docker config files inside Scripts directory, docker is not allowed to get access for ../source so at least compose file must be in the root directory.

@ia
Copy link
Collaborator Author

ia commented Jun 27, 2023

Just checked that content of the site directory (which is generated by make docs) is fully equivalent (not counting time-stamps and actualization of path to TS100 flash script) to gh-pages branch content which is - as far as I understand - used for RtD online IronOS documentation.

start_dev.sh Show resolved Hide resolved
Makefile Outdated

# command for docker-compose tool
ifndef DOCKER_COMPOSE
DOCKER_COMPOSE:=docker-compose
Copy link
Owner

Choose a reason for hiding this comment

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

While we are here, can we test for docker-compose and if its not here fall back to docker compose

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thoughtful suggestion. No problem, I will test & check how to make it in a more or less reliable way.

Copy link
Owner

@Ralim Ralim left a comment

Choose a reason for hiding this comment

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

Hia,
Thank you for the changes; I agree with all of this. Except really want to keep the single script to get into dev container part of the workflow. We shouldn't_ rely on users having make on their system.

@ia
Copy link
Collaborator Author

ia commented Jun 28, 2023

A few questions while I will be working on proposed changes.

  • should I keep or remove Bootup Logo NOTICE in Key Features section from main Readme?

  • is my understanding correct that PULL_REQUEST_TEMPLATE.md file is used only once "statically" - you just configure the project on the github and specify the location/name of template for pull request so this file can have any [persistent] name/location because you use it for configuration only once?

  • is there any way to check changes in .github/workflows/docs.yml to make sure that RtD docs will be generated & published properly? As I mentioned the command mkdocs build -f scripts/IronOS-mkdocs.yml -d ../site from the main root project dir generates site successfully but I would like to make sure that there won't be any breaking changes on publish docs online after this modification.

  • and since I'm not a DevOps person but I'm just curious... so, is my understanding (due to little research) correct that we can't just move yaml config file for docker-compose anywhere else besides main root project dir since it's technical limitations of docker due to mappings of top-level dirs inside of a container? TL;DR: since we need to "map" source/ dir inside docker container, docker-compose.yml file must be on the same level (or higher) than source/ dir which we are mapping, right?

@Ralim
Copy link
Owner

Ralim commented Jun 28, 2023

A few questions while I will be working on proposed changes.

* should I keep or remove `Bootup Logo NOTICE` in `Key Features` section from [main Readme](https://github.com/ia/IronOS-plus/blob/root-dir/README.md#key-features)?

Prefer to keep it; would rather verbosity than yet more tickets

* is my understanding correct that `PULL_REQUEST_TEMPLATE.md` file is used only once "statically" - you just configure the project on the github and specify the location/name of template for pull request so this file can have any [persistent] name/location because you use it for configuration only once?

That file has been here since before you had any customisation control lol. This can likely be refactored into the .github folder. You cant pick the file location itself in the github ui (or I cant find it), but I believe it searches a few spots.

* is there any way to check changes in [`.github/workflows/docs.yml`](https://github.com/ia/IronOS-plus/blob/49366e44457a0f66ca8204a48310b99deb0d9420/.github/workflows/docs.yml#L40) to make sure that RtD `docs` will be generated & published properly? As I mentioned the command `mkdocs build -f scripts/IronOS-mkdocs.yml -d ../site` from the main root project dir generates `site` successfully but I would like to make sure that there won't be any breaking changes on publish docs online after this modification.

Honestly no idea, personally happy to ship this and review (and fix) if it breaks. You have done about all I would do tbh

* and since I'm not a DevOps person but I'm just curious... so, is my understanding (due to little research) correct that we can't just move yaml config file for `docker-compose` anywhere else besides main root project dir since it's technical limitations of docker due to mappings of top-level dirs inside of a container? TL;DR: since we need to "map" `source/` dir inside docker container, docker-compose.yml file must be on the same level (or higher) than `source/` dir which we are mapping, right?

I believe that for security reasons you cant walk the tree above where you start. The convention is that the Dockerfile and docker-compose.yaml are in the root of the source tree.

@ia
Copy link
Collaborator Author

ia commented Jun 28, 2023

Prefer to keep it; would rather verbosity than yet more tickets

No problem at all! Just wanted to make sure since we update it anyway.

This can likely be refactored into the .github folder.

Oh, actually you're right! I should ask google/look for github documentation first before wasting your time with this simple question ;)
There is a detailed instruction of how to do exactly that indeed!

personally happy to ship this and review (and fix) if it breaks

Deal!

I believe that for security reasons you cant walk the tree above where you start. The convention is that the Dockerfile and docker-compose.yaml are in the root of the source tree.

Just like I thought based on the information which I could find. Thanks.

@ia
Copy link
Collaborator Author

ia commented Jun 30, 2023

UPDATE

Main locations/files modifications:

  • move text from Bootup Logo/README.md to README.md using footnote
  • replace build.sh & start_dev.sh by scripts/deploy.sh: run by default starts docker shell, supports build & clean subcommands as first argument (i.e., scripts/deploy.sh build), detects docker/docker-compose or use custom command provided by user;
  • move community script to flash TS100 from Flashing one file directory to newly created scripts directory
  • move Dockerfile to scripts/IronOS.Dockerfile
  • rename docker-compose.yml to IronOS.yml
  • move ci directory, mkdocs.yml, and LICENSE_RELEASE inside scripts directory
  • PULL_REQUEST_TEMPLATE.md and SECURITY.md are placed under the rug to .github directory
  • update & modify related configuration files according to changes
  • top-level Makefile can run docker-related commands, generate docs and pass-through targets for source/ directory; by default (make) it shows help and further instructions
  • since we clean up structure anyway, I took some bravery to sort out files inside Development Resources as well: as far as I understand, there are no any direct references inside of this repo to the files inside of Development Resources directory so I put files to respective sub-dirs

What have been tested:

Two main concerns are:

  • online docs generation
  • out-of-dated references to moved/changed files/scripts from other places which I'm not aware of

But I guess these two will be fixed once we discover them. :)

@ia
Copy link
Collaborator Author

ia commented Jun 30, 2023

Oh, I see some shellcheck problems. Will try to fix it. Seems done.

@ia
Copy link
Collaborator Author

ia commented Jun 30, 2023

Another one testing result update: I can confirm that command mkdocs gh-deploy -f scripts/IronOS-mkdocs.yml -d ../site automagically generates & deploys online documentation successfully for my cloned repo with current changes in root-dir/Documentation such as new path for TS100 flash script but prefix for those links in the docs leads to the original repo since it's hard-coded in the documents so there is no any issue with that.

@ia
Copy link
Collaborator Author

ia commented Jun 30, 2023

Just to clarify: duplicating the functions of Makefile and deploy.sh is not a bug but a feature since we try to satisfy both [probably/presumably] mutually exclusive environments:

  • deploy.sh when everything needed is just a shell & docker installed (no make and other fancy toolchain-related packages)
  • Makefile when besides make & docker such tools as python/mkdocs/etc. are presented

It could be logical to make them rely one on each other (for example, calling deploy.sh from Makefile) but I would like to keep them independently & separately as self-efficient.

@ia
Copy link
Collaborator Author

ia commented Jul 1, 2023

Wiki documentation has been refreshed & updated as well. Ironically, but despite the fact that wiki is managed by git repo on github, there is no way to make a pull request using web interface AFAIK. So, repo with some updates is here and the way how it will be looking is here. The original repo IronOS.wiki.git has been cloned & forked before I pushed the changes, so there shouldn't be any git-related problems to merge it back into IronOS.wiki.git. I guess it should be something like:

  • clone upstream wiki: git clone Ralim/IronOS.wiki.git
  • add remote repo with updates: git remote add fork https://github.com/ia/IronOS-plus.wiki.git
    and then something like:
$ git fetch fork
$ git checkout -b update fork/master
$ git checkout -b master origin/master
$ git merge update
$ git commit -m "merge wiki update"
$ git push

Yes, I know that ReadTheDocs online docs getting more often updates and probably will be the only one source of documentation soon, but since wiki is still there [yet] & it's "googl-able", I thought it should be updated as well just in case.

@Ralim
Copy link
Owner

Ralim commented Jul 2, 2023

I'm 100% happy with this given the following:
(1) I'm really not going to dig too heavily into the new Makefile, it looks good but I'm not a guru

Ergo, I'm going to merge this ^_^; suuper thankful for all of this work btw.


My eventual goal is to fully remove the wiki if I can and just point everyone to do the read-the-docs.
Would it be better to just migrate any leftover information in it now and annex it? (open to opinions here).

@Ralim Ralim merged commit b524a99 into Ralim:dev Jul 2, 2023
@Ralim
Copy link
Owner

Ralim commented Jul 2, 2023

Have applied wiki updates as well (fwiw)

@ia
Copy link
Collaborator Author

ia commented Jul 2, 2023

I'm really not going to dig too heavily into the new Makefile, it looks good but I'm not a guru

Really? But source/Makefile looks very sophisticated & impressive!

Ergo, I'm going to merge this ^_^

Since I'm the initiator for that, I don't mind to be responsible for top-level Makefile & scripts/deploy.sh and their further maintenance/fixes/updates (if any), so once if you see anyone having any problems with that, just "mention"/ping me back right away without any hesitation. :)

suuper thankful for all of this work btw.

Thank you for accepting my patches. 🙏

My eventual goal is to fully remove the wiki

I agree that there is no much of the point to have two different docs "engines" for the same project.

if I can and just point everyone to do the read-the-docs.

I think I have some idea...

Would it be better to just migrate any leftover information in it now and annex it? (open to opinions here).

I will add this to my todo list. When I have time & fresh head, I will try to look through content from wiki very carefully again and if there will be something relevant worth saving info, then I will be happy to move it to Documentation/. In fact, yes, I personally think it will be way much more better even from the point of updating & maintaining documentation files - since Documentation/ is a part of repo, it gives all the github's whistles & bells to manage it while wiki files are "hidden" from such luxury. :D

Oh, and it's always just a pleasure to interact with you & others in comments for PRs & Issues of this project. Thanks a lot for such communal hospitality especially for new comers & newbie contributors. 🥹

@ia
Copy link
Collaborator Author

ia commented Jul 2, 2023

Have applied wiki updates as well (fwiw)

Could you, please, make just only one more wiki merge?
I like the idea adding "wiki probable discontinue" notice. So here is how it looks now. Should be enough not to touch wiki anymore for now, and we don't have to worry for users reading out-of-dated information there.

@ia ia mentioned this pull request Jul 4, 2023
2 tasks
@Ralim
Copy link
Owner

Ralim commented Jul 5, 2023

Wiki updated

@ia ia deleted the root-dir branch July 23, 2023 21:35
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.

3 participants