Skip to content

Conversation

@Remi-Gau
Copy link
Contributor

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Bug fix

PR Summary: This pull request aims to address the installation process of the MACS toolbox during the initial setup by removing the steps that copy the MACS toolbox to the SPM inputs folder across various GitHub workflow files. The intention appears to streamline the setup process or integrate the toolbox installation in a different manner not visible in the provided diff. Additionally, an entry is added to the CHANGELOG under the 'Fixed' section, indicating an attempt to rectify the toolbox installation process.

Decision: Comment

📝 Type: 'Bug fix' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
  • Unsupported files: the diff contains files that Sourcery does not currently support during reviews.

General suggestions:

  • Ensure that the removal of the MACS toolbox copy steps does not inadvertently affect the functionality or dependencies required by the workflows. It might be beneficial to include a brief explanation in the PR description or comments to clarify how the MACS toolbox will be handled moving forward.
  • The changelog entry mentions fixing the copy process of the MACS toolbox, which seems to contradict the removal of the copy steps. It's recommended to resolve this discrepancy to maintain clarity. If the installation process has been modified or relocated, consider updating the changelog to accurately reflect these changes.
  • Given the removal of specific steps across multiple workflow files, it's crucial to verify that all tests and operations that previously depended on the MACS toolbox still function as expected. If the toolbox is now installed or integrated differently, confirming that this new method is reflected in all relevant documentation and workflow configurations would be prudent.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Apr 4, 2024

To do in a separate PR

Fix Octave related things.

@Remi-Gau Remi-Gau merged commit 5c8fec5 into cpp-lln-lab:main Apr 4, 2024
@Remi-Gau Remi-Gau deleted the fix/1202 branch April 4, 2024 10:01
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.

[BUG] MACS toolbox is not properly initialise in spm (copied to toolbox folder?)

1 participant