feat(factory): add factory package

[grandwizard28]

Summary

• add factory package

Related Issues / PR's

Part of #6782

---

Important

Introduces a new factory package with interfaces and implementations for configuration, provider, and service factories, along with comprehensive tests.

• Factory Package:
  • Introduces Config, ConfigFactory, ProviderFactory, and Service interfaces in config.go, provider.go, and service.go.
  • Implements NamedMap for managing named factories in named.go.
  • Adds ProviderSettings and ScopedProviderSettings for managing provider settings in setting.go.
• Tests:
  • Adds unit tests for ConfigFactory in config_test.go.
  • Adds unit tests for Name and NamedMap in name_test.go and named_test.go.
  • Adds unit tests for ProviderFactory in provider_test.go.
  • Implements httpService for testing Service interface in servicetest/http.go.

^{This description was created by for 962b142. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to bff040c in 4 minutes and 5 seconds

More details

• Looked at 348 lines of code in 7 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. pkg/factory/config.go:29

• Draft comment:
  The comment incorrectly states that this function creates a new config. It should describe that this function returns a new config instance using the factory's function.
• Reason this comment was not posted:
  Marked as duplicate.

2. pkg/factory/config.go:34

• Draft comment:
  The comment should start with the function name, e.g., "NewConfigFactory creates a new config factory."
• Reason this comment was not posted:
  Confidence changes required: 50%
  The comment on line 34 of config.go should be updated to follow Go's convention for function comments, starting with the function name.

3. pkg/factory/factorytest/http.go:19

• Draft comment:
  The error return value is not used in this function. Consider removing it if it's not needed.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  This is a constructor function for a service that implements factory.Service interface. While the error is currently unused, having error returns in constructors is a common Go pattern to allow for future error handling. Removing it could make future changes harder. The comment is technically correct but may not be the best practice to follow.
  The comment is technically accurate - the error is unused. But am I considering enough Go idioms and best practices?
  In Go, it's idiomatic for constructors to return (Type, error) even if errors aren't currently returned, as it provides flexibility for future changes without breaking the API.
  The comment should be deleted. While technically correct, following it would go against Go best practices for constructor functions.

4. pkg/factory/factorytest/http.go:30

• Draft comment:
  Consider closing the listener in a defer statement to ensure it is closed when the function exits, preventing resource leaks.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:

1. The http.Server.Serve() method takes ownership of the listener. 2. When http.Server.Shutdown() is called, it automatically closes the underlying listener. 3. The Stop method already handles cleanup through Server.Shutdown(). 4. Adding an explicit listener.Close() could interfere with the server's normal shutdown process.
   The comment might be pointing out a real concern about resource cleanup, and explicit cleanup is often good practice.
   In this case, the http.Server already handles listener cleanup as part of its standard shutdown process, making an explicit close unnecessary and potentially harmful.
   The comment should be deleted because the listener cleanup is already handled correctly by http.Server.Shutdown() in the Stop method.

5. pkg/factory/provider.go:35

• Draft comment:
  Consider returning a more descriptive error message if provider creation fails, to aid in debugging.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The error being returned comes from the provider.New() call, which itself is just passing through an error from the newProviderFunc. This function is just a thin wrapper/factory layer. Adding error context here would be redundant since the actual error details should be handled by the concrete provider implementations. The error handling pattern used is idiomatic Go.
  The comment could have a point - more context in errors can help debugging. Maybe wrapping the error with factory details would help trace the error path.
  This is a generic factory pattern that just propagates errors. Error details should be handled by the concrete implementations. Adding error wrapping here would violate separation of concerns.
  The comment should be deleted. The error handling is appropriate for this abstraction layer and any error details should be handled by the concrete provider implementations.

6. pkg/factory/factorytest/http.go:1

• Draft comment:
  Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is applicable to the overall file structure in this PR.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_cdqMbvGvEwAJewa6

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[nityanandagohain]

minor comments

[ellipsis-dev]

👍 Looks good to me! Incremental review on aee556d in 20 seconds

More details

• Looked at 124 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. pkg/factory/named.go:36

• Draft comment:
  Changing the receiver from a value to a pointer for Get, Add, and GetInOrder is a good practice as it avoids unnecessary copying of the NamedMap struct and ensures modifications affect the original instance.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The method receiver for Get, Add, and GetInOrder in NamedMap was changed from a value receiver to a pointer receiver. This is a good change because it avoids unnecessary copying of the NamedMap struct, especially since these methods modify the struct (Add) or are expected to work on the same instance (Get, GetInOrder).

2. pkg/factory/named_test.go:1

• Draft comment:
  Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This applies to the pkg/factory/named_test.go file.
• Reason this comment was not posted:
  Comment did not seem useful.

Workflow ID: wflow_XzlgNNIkFZoJYP0g

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on db708f7 in 44 seconds

More details

• Looked at 108 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 4 drafted comments based on config settings.

1. pkg/factory/config_test.go:1

• Draft comment:
  Consider adding a package documentation comment to describe the purpose of this test file.
• Reason this comment was not posted:
  Confidence changes required: 20%
  The test files are missing package documentation comments. It's a good practice to include them for clarity and maintainability.

2. pkg/factory/name_test.go:1

• Draft comment:
  Consider adding a package documentation comment to describe the purpose of this test file.
• Reason this comment was not posted:
  Confidence changes required: 20%
  The test files are missing package documentation comments. It's a good practice to include them for clarity and maintainability.

3. pkg/factory/provider_test.go:1

• Draft comment:
  Consider adding a package documentation comment to describe the purpose of this test file.
• Reason this comment was not posted:
  Confidence changes required: 20%
  The test files are missing package documentation comments. It's a good practice to include them for clarity and maintainability.

4. pkg/factory/config_test.go:1

• Draft comment:
  Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This applies to all test files in this PR.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_jmtR9Us4czTwDDwU

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 7a98965 in 13 seconds

More details

• Looked at 12 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_r2lKhm0vhDzwwOdr

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 962b142 in 14 seconds

More details

• Looked at 48 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. pkg/factory/named.go:12

• Draft comment:
  The import statement for "fmt" is unnecessary as it is not used in this file. Consider removing it to clean up the code.
• Reason this comment was not posted:
  Confidence changes required: 20%
  The import statement for "fmt" is unnecessary in this file as it is not used anywhere in the code. Removing it will clean up the code.

Workflow ID: wflow_e5CzVlKvRnJ8PPjs

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[nityanandagohain]

Try not to use force push.