From f225a25782249bb44e1ba5076c9cb3d800ccf939 Mon Sep 17 00:00:00 2001 From: raoulvdberge Date: Wed, 28 Dec 2022 12:44:38 +0100 Subject: [PATCH] Add ADRs. See #218 --- .github/CONTRIBUTING.md | 17 ++++--- .../decision/001-multi-loader-architecture.md | 40 ++++++++++++++++ doc/architecture/decision/002-api-modules.md | 47 +++++++++++++++++++ .../decision/003-the-platform-api-module.md | 39 +++++++++++++++ .../decision/004-coverage-requirements.md | 42 +++++++++++++++++ doc/architecture/decision/005-unit-testing.md | 39 +++++++++++++++ .../decision/006-no-persistent-networks.md | 41 ++++++++++++++++ .../node/exporter/ExporterNetworkNode.java | 1 - 8 files changed, 258 insertions(+), 8 deletions(-) create mode 100644 doc/architecture/decision/001-multi-loader-architecture.md create mode 100644 doc/architecture/decision/002-api-modules.md create mode 100644 doc/architecture/decision/003-the-platform-api-module.md create mode 100644 doc/architecture/decision/004-coverage-requirements.md create mode 100644 doc/architecture/decision/005-unit-testing.md create mode 100644 doc/architecture/decision/006-no-persistent-networks.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index e112ce205..b3a776a9f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -1,8 +1,5 @@ # Contributing to Refined Storage -> This project won't accept any code contributions at this time. The repository has been made public for -> transparency, but it's too early to contribute for now. - ## Versioning This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). @@ -35,7 +32,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). This project uses [Gitflow](https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow). -We use [support branches](https://groups.google.com/g/gitflow-users/c/I9sErOSzYzE/m/AwVH06CuKT0J) for supporting different major/minor versions at the same time. +We use [support branches](https://groups.google.com/g/gitflow-users/c/I9sErOSzYzE/m/AwVH06CuKT0J) for supporting +different major/minor versions at the same time. ## Documentation @@ -47,6 +45,12 @@ We use [Checkstyle](https://checkstyle.sourceforge.io/) in our build pipeline to It is recommended to import the `checkstyle.xml` config into your IDE and/or install a Checkstyle plugin. +## Architecture + +### Architecture Decision Records + +The ADRs of this project can be found under [doc/architecture/decision](../doc/architecture/decision). + ## Testing When adding functionality or fixing a bug, it is important to add tests. Tests are important, if not more important, @@ -108,9 +112,9 @@ After that, a Discord and Twitter notification is sent. ## Modules -Refined Storage 2 is modularized. That means that the project is split up into various modules. +Refined Storage 2 is modularized, the project is split up into various modules. -Important to note is that most modules aren't dependent on Minecraft or a mod loader. Only modules that start +Most modules aren't dependent on Minecraft or a mod loader. Only modules that start with `refinedstorage2-platform-*` have dependencies on Minecraft. | Name | Use in addons | Description | @@ -118,7 +122,6 @@ with `refinedstorage2-platform-*` have dependencies on Minecraft. | `refinedstorage2-core-api` | ✔️ | Contains some utility classes and enums. | | `refinedstorage2-grid-api` | ✔️ | Contains Grid related functionality. | | `refinedstorage2-network-api` | ✔️ | Contains storage network related functionality. | -| `refinedstorage2-network-api` | ✔️ | Contains storage network related functionality. | | `refinedstorage2-network-test` | ✔️ | JUnit extension which helps with setting up a network and a network node for testing. | | `refinedstorage2-query-parser` | ✔️ | A query parser, contains a lexer and parser. Only used for Grid query parsing. | | `refinedstorage2-resource-api` | ✔️ | Contains API for handling resources. | diff --git a/doc/architecture/decision/001-multi-loader-architecture.md b/doc/architecture/decision/001-multi-loader-architecture.md new file mode 100644 index 000000000..d0a6bc5d0 --- /dev/null +++ b/doc/architecture/decision/001-multi-loader-architecture.md @@ -0,0 +1,40 @@ +# 1. Multi-loader architecture + +Date: 2021-12-21 + +## Status + +Accepted + +## Context + +A big reason for rewriting Refined Storage is to have support for multiple modloaders. Therefore, the architecture +should support this design goal in a maintainable way. + +## Decision + +Refined Storage 2 will have a modular structure with a module per modloader. + +There will be a common module, and most code should reside in there. The modloader specific (platform) modules should be +reserved +for platform specific functionality, like registration, networking, etc. + +The module naming follows the standard of `refinedstorage2-platform-{name}`. + +## Consequences + +Thanks to the modular structure, it will be easier to separate modloader specific code and Minecraft code. + +Apart from the structural benefits, it will be easier to maintain support for different modloaders as opposed to working +with multiple Git branches and rebasing (or copying code over). + +The build system will become more complicated, as the common module needs to be "loaded" into the specific modloader ( +platform) +modules. + +It will also be harder to update the mod to a newer Minecraft version if not all our supported modloaders are upgraded +yet. However, in that case we can probably disable the specific platform module in the build system. + +## References + +- https://github.com/refinedmods/refinedstorage2/issues/64 \ No newline at end of file diff --git a/doc/architecture/decision/002-api-modules.md b/doc/architecture/decision/002-api-modules.md new file mode 100644 index 000000000..2010edc9a --- /dev/null +++ b/doc/architecture/decision/002-api-modules.md @@ -0,0 +1,47 @@ +# 2. API modules + +Date: 2021-08-21 + +## Status + +Accepted + +## Context + +Most of the business logic in Refined Storage 1 is interweaved with Minecraft specific code. + +This makes it harder to write unit tests and to maintain the codebase. + +Along with the negative impact on testability, this makes our business logic prone to changes in Minecraft. Ideally, our +business logic shouldn't change due to a Minecraft update. + +## Decision + +Refined Storage 2 will have multiple API modules that have no dependency on Minecraft whatsoever. +These modules can depend on each other, but the dependency chain should be logical. + +These API modules can be consumed by platform modules (to bundle them) [[1]](#1) and addon developers. + +The platform modules [[1]](#1) are responsible for integrating the modules with Minecraft, by implementing interfaces on +their end that the API modules can use. + +The API modules have the name `refinedstorage2-{name}-api`. + +## Consequences + +Using multiple API modules ties in nicely with our modular structure for platform modules [[1]](#1). + +It will be difficult to integrate these modules with Minecraft code, so designing them should be done thoughtfully. + +It will be easier to maintain and unit test these modules as they have no dependency on Minecraft. + +## References + +- [1] See [1. Multi-loader architecture](001-multi-loader-architecture.md) +- https://github.com/refinedmods/refinedstorage2/commit/42cd440dfa227673be88917193a5327285d85f47 +- https://github.com/refinedmods/refinedstorage2/commit/a3fce1c12bc5e1d06db4441a78b9edc56852f2bc +- https://github.com/refinedmods/refinedstorage2/commit/d46fba9bed46edaa3c7ed57fc780b10d724c4a2c +- https://github.com/refinedmods/refinedstorage2/commit/5b798047e9573e787e071e14fe7de122218389f9 +- https://github.com/refinedmods/refinedstorage2/commit/66416a25f0c32127c94aed265c5e62a2b4374f31 +- https://github.com/refinedmods/refinedstorage2/commit/bcf3324b71ea837fb044b051b8d5bb9c9e108028 +- https://github.com/refinedmods/refinedstorage2/commit/1b86e4364561160ceda9a62f6f5e10298d989791 diff --git a/doc/architecture/decision/003-the-platform-api-module.md b/doc/architecture/decision/003-the-platform-api-module.md new file mode 100644 index 000000000..58349b1ef --- /dev/null +++ b/doc/architecture/decision/003-the-platform-api-module.md @@ -0,0 +1,39 @@ +# 3. The platform API module + +Date: 2021-12-29 + +## Status + +Accepted + +## Context + +Now that we have platform modules (one for each modloader, and a common module) [[1]](#1) and API +modules [[2]](#2), we still need a way for addons to integrate with the mod. + +Doing so via the common module would be detrimental as it would expose too much code to addons (just like it did in +Refined Storage 1). + +## Decision + +Refined Storage 2 will have a modloader-neutral (just like the common module) platform API module which addon mods can +use to integrate with Refined Storage. + +Moreover, Refined Storage 2 itself will use this platform API module. + +The module is named `refinedstorage2-platform-api`. + +## Consequences + +By offering a dedicated platform API module we can much more tightly control API surface. + +The platform API module is platform-neutral so addons can decide what modloader they target. Moreover, it saves us time +because we don't have to maintain different platform APIs per modloader. + +However, if addon mods want to support multiple modloaders, they'll have to create their own abstractions. + +## References + +- [1] See [1. Multi-loader architecture](001-multi-loader-architecture.md) +- [1] See [2. API modules](002-api-modules.md) +- https://github.com/refinedmods/refinedstorage2/commit/64e97fd170185d3d55a60879db7fca2134ae6dd0 \ No newline at end of file diff --git a/doc/architecture/decision/004-coverage-requirements.md b/doc/architecture/decision/004-coverage-requirements.md new file mode 100644 index 000000000..167f407f7 --- /dev/null +++ b/doc/architecture/decision/004-coverage-requirements.md @@ -0,0 +1,42 @@ +# 4. Coverage requirements + +Date: 2021-12-22 + +## Status + +Accepted + +## Context + +Since we are writing more unit tests with our API modules [[1]](#1) we need to establish code coverage requirements. + +## Decision + +We will differentiate between platform modules [[2]](#2) and API modules [[1]](#1). The reason for this is that it's difficult to test +Minecraft code properly. + +Luckily, since our most important (business-logic) code resides in the API modules [[1]](#1) we can put +our testing focus there and establish coverage requirements. + +However, sometimes it is still advised to write a test for platform code, even if there are no coverage requirements for +it. For that reason, the `refinedstorage2-platform-test` module exists, to provide helpers to deal with Minecraft code. + +## Consequences + +For API modules, this means that: + +- Minimum 80% code coverage +- Minimum 90% mutation coverage + +For platform modules, this means that: + +- No code coverage requirement +- No mutation coverage requirement + +These requirements will be enforced by SonarQube and Pitest. + +## References + +- [1] See [2. API modules](002-api-modules.md) +- [2] See [1. Multi-loader architecture](001-multi-loader-architecture.md) +- https://github.com/refinedmods/refinedstorage2/issues/115 diff --git a/doc/architecture/decision/005-unit-testing.md b/doc/architecture/decision/005-unit-testing.md new file mode 100644 index 000000000..b1fc190fc --- /dev/null +++ b/doc/architecture/decision/005-unit-testing.md @@ -0,0 +1,39 @@ +# 5. Unit testing + +Date: 2020-10-24 + +## Status + +Accepted + +## Context + +The coverage requirements [[1]](#1) for the API modules [[2]](#2) state that we need unit test coverage. Hence, we need +to clarify what "unit" in unit test means. + +The API modules in Refined Storage 2 are already disconnected from reality (Minecraft), so we need unit tests that +closely mimick real situations and assert real behavior if we want to get any value out of them. + +This eliminates the definition that most +people have of unit tests, which is to test "a single method" and to mock dependencies of the class of that method. + +## Decision + +Refined Storage 2 unit testing will follow a behavior driven approach, where "unit" in unit test means "unit of +behavior" and not "unit of method". + +## Consequences + +A unit test should verify behavior and not make assumptions about internal code structure, therefore most use of mocking +is not allowed. This will make our test suite stronger and less prone to breakage. + +Our tests will not break due to a refactoring. + +This doesn't mean that we are suddenly "integration testing", just because we decide to not mock dependencies of +classes. Integration testing will happen in the platform modules [[3]](#3) as Minecraft game tests. + +## References + +- [1] See [4. Coverage requirements](004-coverage-requirements.md) +- [2] See [2. API modules](002-api-modules.md) +- [3] See [1. Multi-loader architecture](001-multi-loader-architecture.md) \ No newline at end of file diff --git a/doc/architecture/decision/006-no-persistent-networks.md b/doc/architecture/decision/006-no-persistent-networks.md new file mode 100644 index 000000000..37a5588d7 --- /dev/null +++ b/doc/architecture/decision/006-no-persistent-networks.md @@ -0,0 +1,41 @@ +# 6. No persistent networks + +Date: 2021-05-24 + +## Status + +Accepted + +## Context + +Networks and network nodes were persisted seperately in Refined Storage 1. + +This caused a lot of problems, because there was a disconnect between the real state of the world and the persisted +file. + +Most of the time, this didn't lead to any problems, but it quickly failed with block moving mods or mods that worked +with block entities in non-conventional ways. + +## Decision + +Networks cannot be persisted in Refined Storage 2. They need to be loaded in memory. + +Network nodes cannot be persisted in Refined Storage 2. All persistent data should be stored on corresponding the block +entity. + +## Consequences + +It will no longer be possible to persist global network data, since there are no persistent networks. + +This means that some persistent components like resource trackers (when has what been changed in het network?) need to +be stored on the storage itself. + +Since networks are no longer persisted and built ad-hoc, it should make the mod more robust. Also, it allows to have +multiple entry-points to a network. + +For network nodes, storing everything on the block entity should simplify the code a lot. There is simply no need for +decoupling the storage part, and just causes friction like in Refined Storage 1. + +## References + +- https://github.com/refinedmods/refinedstorage2/commit/df8f881c1c300e1cb6f02a8ac3f03aa163f634db diff --git a/refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/node/exporter/ExporterNetworkNode.java b/refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/node/exporter/ExporterNetworkNode.java index 97b52e804..703a1a22e 100644 --- a/refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/node/exporter/ExporterNetworkNode.java +++ b/refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/node/exporter/ExporterNetworkNode.java @@ -11,7 +11,6 @@ import javax.annotation.Nullable; // TODO: Move to own module (refinedstorage2-network), everything in api should have @Api annot. -// TODO: Add ADRs public class ExporterNetworkNode extends AbstractNetworkNode { private long energyUsage; private final Actor actor = new NetworkNodeActor(this);