Skip to content

Conversation

@omnom62
Copy link
Contributor

@omnom62 omnom62 commented Feb 12, 2025

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T7012

Related PR(s)

Component(s) name

Proposed changes

How to test

Test results

$ ansible-test units --docker -v --python 3.12

  • generated xml file: /root/ansible_collections/vyos/vyos/tests/output/junit/python3.12-controller-units.xml -
    ============================= 345 passed in 10.05s =============================
  • Sanity tests passed
  • Unit tests passed

Tested against VyOS versions:

  • 1.3.8
  • 1.4-rolling-202201010100
  • 1.5-rolling-202502120007

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the ansible sanity and unit tests
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added unit tests to cover my changes
  • I have added a file to changelogs/fragments to describe the changes

@omnom62 omnom62 marked this pull request as ready for review February 18, 2025 20:30
@omnom62 omnom62 requested a review from a team as a code owner February 18, 2025 20:30
@omnom62 omnom62 requested review from andamasov, gaige and zdc February 18, 2025 20:30
Copy link
Contributor

@gaige gaige left a comment

Choose a reason for hiding this comment

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

Appears to be missing at least one file update in parsed. A few other requests, otherwise looking good.

@omnom62 omnom62 requested a review from gaige February 19, 2025 11:39
@omnom62
Copy link
Contributor Author

omnom62 commented Feb 19, 2025

Added pre- and post-tasks script to add target dependencies

@gaige
Copy link
Contributor

gaige commented Feb 20, 2025

Can you update the PR template here? The test versions seem out of date, there's no link to the story.

Copy link
Contributor

@gaige gaige left a comment

Choose a reason for hiding this comment

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

Tests look good now for me. However, please verify that pre-commit is running on your system and then manually re-run it (per slack) to update wrapping, etc. Also update the PR information in the template .

@omnom62
Copy link
Contributor Author

omnom62 commented Feb 20, 2025

Can you update the PR template here? The test versions seem out of date, there's no link to the story.

done

Copy link
Contributor

@gaige gaige left a comment

Choose a reason for hiding this comment

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

One concern here, but otherwise good to go. Tests succeed reliably on my systems.

@omnom62 omnom62 requested a review from gaige March 2, 2025 22:14
@omnom62 omnom62 requested a review from a team March 2, 2025 23:13
Copy link
Contributor

@gaige gaige left a comment

Choose a reason for hiding this comment

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

Tests pass, looks good.

@omnom62 omnom62 requested review from a team and vyosbot and removed request for a team March 9, 2025 19:42
Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

My only note is that VyOS 1.3 is EOL now and all its mentions can be removed/omitted in the future, I think.

@gaige
Copy link
Contributor

gaige commented Apr 2, 2025

My only note is that VyOS 1.3 is EOL now and all its mentions can be removed/omitted in the future, I think.

Yeah, the plan (detailed in the README.md) is to remove support for 1.3.8 in 7.0.x. Considering that 6.0.0 has much more test coverage, we want to get it out supporting 1.3.8, so that there's a stable release for folks to move from 1.3.8 to 1.4.x+ (since that would mean they can gather and send configs.

@gaige gaige merged commit 640519a into vyos:main Apr 2, 2025
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants