Skip to content

Update fxp.yaml#729

Closed
ahmedshahein wants to merge 3 commits intognu-octave:mainfrom
ahmedshahein:patch-4
Closed

Update fxp.yaml#729
ahmedshahein wants to merge 3 commits intognu-octave:mainfrom
ahmedshahein:patch-4

Conversation

@ahmedshahein
Copy link
Copy Markdown
Contributor

Added comprehensive help, “help fxp.
Added self-checking testing using 28 tests, “test fxp”.
Bug fix in rounding scheme “fix – round towards zero”.

Added comprehensive help, “help fxp.
Added self-checking testing using 28 tests, “test fxp”.
Bug fix in rounding scheme “fix – round towards zero”.
Fix fxp.yaml by adding depends section.
@ahmedshahein
Copy link
Copy Markdown
Contributor Author

This is further enhancements and updates.

  • Added comprehensive help, “help fxp.
  • Added self-checking testing, “test fxp”.
  • Bug fix in rounding scheme “fix – round towards zero”.
    I have a remark regarding the installation using the standard "pkg install -forge fxp". I observed that despite it fetches the correct file the installation path is still using the initial file. Despite that I manually removed the folder it still coming back to the old path, as shown below:
    .var/app/org.octave.Octave/data/octave/api-v60/packages/fxp-1.0.1/

Could you please check this issue?
Regards,
Ahmed.

@mmuetzel
Copy link
Copy Markdown
Member

mmuetzel commented Apr 6, 2026

  • I have a remark regarding the installation using the standard "pkg install -forge fxp". I observed that despite it fetches the correct file the installation path is still using the initial file. Despite that I manually removed the folder it still coming back to the old path, as shown below:
    .var/app/org.octave.Octave/data/octave/api-v60/packages/fxp-1.0.1/

I'm not sure I correctly understand what you mean. But it looks like you forgot to update the DESCRIPTION file. It still says:
https://github.com/ahmedshahein/pkg-fxp/blob/e07b5b6ec59c228e610c2d82f9ff85609e646eb3/DESCRIPTION#L2-L3

Version: 1.0.1
Date: 2026-01-26

Agree on stating only the minimum tested version of the tool.

Co-authored-by: Markus Mützel <markus.muetzel@gmx.de>
Copy link
Copy Markdown
Contributor Author

@ahmedshahein ahmedshahein left a comment

Choose a reason for hiding this comment

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

Accepted suggestions of stating on the minimum (least) tested tool version.

@mmuetzel
Copy link
Copy Markdown
Member

mmuetzel commented Apr 11, 2026

If you don't mind the wrong version being reported, e.g., for pkg list, we could merge the update as-is.
Otherwise, you should fix the DESCRIPTION file in your repository and prepare a new tarball for your package.

Please, let us know how you'd like to proceed.

@ahmedshahein
Copy link
Copy Markdown
Contributor Author

Hi,
I have just updated the DESCRIPTION file as advised.
Thanks

@mmuetzel
Copy link
Copy Markdown
Member

Should we wait for you to update the 3.0.0 tag in your repository (and update the checksum in this PR accordingly)? Or do you prefer to make the release with the tarball before that change (i.e., with the version mismatch)?

@ahmedshahein
Copy link
Copy Markdown
Contributor Author

ahmedshahein commented Apr 11, 2026

The tag (3.0.0) and the checksum was already updated before I initiated the review. Do I need to regenerate the checksum for any reason?

@mmuetzel
Copy link
Copy Markdown
Member

The tag (3.0.0) and the checksum was already updated before I initiated the review. Do I need to regenerate the checksum for any reason?

You wrote that you fixed the version and date in the DESCRIPTION file in your repository. But that was after you created this PR. Hence, that change wouldn't be part of the version that users would be installing in Octave with pkg install fxp if we were to merge this PR as-is.
That is why I asked you whether you would like to move the tag in your repository to a commit that includes the changed DESCRIPTION file.

@ahmedshahein
Copy link
Copy Markdown
Contributor Author

Oh, okay, I understood now.
I prefer to make a new release then with all these fixes. Will do it today.

@mmuetzel
Copy link
Copy Markdown
Member

Closing this PR in favor of #735.

@mmuetzel mmuetzel closed this Apr 12, 2026
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.

2 participants