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

Store project and brick configuration in separate files #315

Closed
tengstrand opened this issue Jul 21, 2023 · 9 comments
Closed

Store project and brick configuration in separate files #315

tengstrand opened this issue Jul 21, 2023 · 9 comments

Comments

@tengstrand
Copy link
Collaborator

tengstrand commented Jul 21, 2023

Note: In the end, we decided to only update the workspace structure, documented here for 0.2.19. When configuration for projects and bricks was moved out of workspace.edn, it became more difficult to get an overview of all configuration, so we decided to keep them there.


Today we store project configuration in workspace.edn in :projects where each project has its own key. A better solution would be to let each project have its own project.edn config file where this information could be stored.

This config file will be stored in the development directory for the development project, and in each project's directory under the projects directory for other projects.

We should move brick configuration that lives in workspace.edn, under the :bricks key (used to ignore files) to separate base.edn and component.edn files under each brick.

We could also move keys under :changes, like :changes > :project-to-bricks-to-test > PROJECT could be moved to :projects > PROJECT > :bricks-to-test.

Consider moving :settings > :profile-to-settings to :profiles and store it in a vector like other entities. Then the code can transform it into a key>value map as today if it simplifies the implementation.

This would also simplify adding support for cljs, see #301.

We can help the user to migrate a workspace, using the migrate command, and showing a warning till that has been done. It has a value that all Polylith workspaces look the same worldwide and that the documentation only talks about this new way of configuring projects. The message that is shown today, when creating a project, can be removed with this change.

When we read old workspaces, :settings > :projects will be converted to the new internal workspace format, where these settings are stored at the project level (the :projects key).
The same goes for :settings > :profile-to-settings which can be stored in a :profiles key in the development project .

The create project command will create a project.edn config file, that looks like this for the development project:

{:alias "dev"}

And for other projects, we can add the alias that we implicitly create today, e.g.:

{:alias "?1"}

When more configuration is added to a project, e.g. :test or :necessary, then they can be added to the project.edn file where they belong.

The test-runner-orchestrator component will be updated, and this change will not affect external test runners.

The keys in base.edn will also be added to :configs > :bases > BASE in the workspace structure.
The keys in component.edn will also be added to :configs > :components > COMPONENT in the workspace structure.
The keys in project.edn will also be added to :configs > :projects > PROJECT in the workspace structure.


When this issue was implemented, I decided to put project and brick configs in config.edn files under each entity. The name of the config file can be set with the :config-filename attribute. See next-release.txt for details.

@tengstrand tengstrand changed the title Store project configuration in project.edn files Store project and brick configuration in separate files Jul 26, 2023
@marksto-workshub
Copy link

This change sounds cool and reasonable! It would also be great if these project.edn and brick.edn files remain open for any custom metadata (extra key-val pairs) that Polylith users are willing to add for their projects.

@tengstrand
Copy link
Collaborator Author

Yes, we need to figure out where custom keys/data should live. A reasonable solution could be to reserve a key, e.g. :custom, where users could put their custom data.

@seancorfield
Copy link
Contributor

My only concern is that in large projects, you are now going from reading one EDN file at the root to potentially hundreds of EDN files spread all over the repository and that's going to have a performance hit I think...

We have ~200 deps.edn files and several operations that poly performs have definitely slowed down as our project has grown.

@tengstrand
Copy link
Collaborator Author

It's true that each project will get another config file, and for you @seancorfield it means 21 more files to parse. When creating a component or base, my idea is to not create a brick.edn config file. The only functionality that needs extra configuration per brick today, is when we add :ignore-files for a brick. When we need that, we will also have to create a new file. This is not so common, so I think the extra config files shouldn't be a performance problem.

@seancorfield
Copy link
Contributor

OK, that's fair enough. Thanks.

tengstrand added a commit that referenced this issue Jan 21, 2024
Add support for storing configuration in config.edn files in projects and bricks, issue #315. The 'migrate' command can be used to automatically migrate a workspace. If a workspace is not migrated, then warning 206 is shown. The internal workspace structure is also affected, and configuration is now stored in each entity (project/brick). Profiles are stored under the :profiles key at the root in the workspace structure. For details, check the next-release.txt at the root of the repository for details. The 'create project' command now supports passing in an alias.
@tengstrand
Copy link
Collaborator Author

The latest 0.2.19-SNAPSHOT release tells people to put their custom data in a :custom key @marksto-workshub.

@seancorfield
Copy link
Contributor

The test-runner-orchestrator component will be updated, and this change will not affect external test runners.

I'll just note that this wasn't true: external test runners were broken by these changes (the changes to the internal structure were reasonable, and both existing external test runners have been updated to work with both internal formats, but more care is needed in the future, as more tools start to depend on the internal workspace data structure).

@seancorfield
Copy link
Contributor

I'm also going to add a note here, reflecting the discussion on Slack that @jasonjckn and I had:

Having migrated to the new EDN file organization at work, I do not like having 20+ separate files to check for poly tool configuration (esp. the per-project aliases), and it's not a great experience in an editor that shows just the filename when you have lots of config.edn files loaded and you can't easily tell which project they belong to.

Just from an overview p.o.v., having to now consult multiple files to look at how the workspace is organized -- or being forced to run a command to get that overview -- is suboptimal.

Having the same name for all these new config files when they have different EDN schemas also feels "wrong".

Also, at work, we've renamed all the generated/migrated config.edn files to polylith.edn so that it is clear that these scattered EDN files belong to the poly tool and not some random per-project configuration/tool.

@tengstrand
Copy link
Collaborator Author

As I answered in the thread in slack, I think we should consider rolling parts of this back, if it makes the user experience worse. We didn't have automated tests that executed the External and Kaocha test runners, and that was why I by mistake introduced a breaking change. Now we have tests in place, that will hopefully stop it from happening again. With tests in place, I guess would not have introduced that breaking change. The reason we continue by fixing the test runners was that it had some value to support the new cleaner workspace structure, where test configuration is stored per project. It will also make it slightly easier to implement test runners in the future.
I will ask the community to see if they want to roll back the use of separate config files per project and brick.

tengstrand added a commit that referenced this issue Feb 10, 2024
Issue #315. Rolled back the use of separate config.edn files in projects and bricks.
* Added test/brick-names-to-test to the clj-poly API.
* Added link to how to the Cursive documentation on how to setup Polylith.
* Stop supporting the migrate command.
* Upgrade edamame (issue #432) and other libraries to the latest version.
* Fixed broken links in the documentation.
* Replaced next-release.txt with next-release.adoc.
* Fixed alignment poblems in the generated cljdoc API doc.
* Added John Shaffer (https://github.com/john-shaffer) as Polylith Fans sponsor. Thanks!
tengstrand added a commit that referenced this issue Feb 10, 2024
Issue #315.
* Updated a warning, telling that the library prefix doesn't need to include .bases or .components.
* Describe how to refresh a workspace from a shell.
* Note that old exported files are automatically migrated.
* Reference Cursive documentation.
* Note that people need to bump to the latest version of test runners, if they use something else than the built-in one.
* Make sure we can create a workspace, even if we have a deps.edn file that we can't read.
* 0.2.19-SNAPSHOT #8
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

No branches or pull requests

3 participants