This repository has been archived by the owner on Sep 7, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add PNG path functionality for IP Fabric v4 #61
Add PNG path functionality for IP Fabric v4 #61
Changes from all commits
4706ca7
0a54269
ba8f2e0
d7b1776
89f2284
256a9cc
03ee356
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks to me that we could implement a generic version validation function, as the product evolves, more cases like this will arise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I'd like to get some feedback on how we could approach version management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feedback from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyone who is currently maintaining a library that is dependent on a specific API/software library and it's versioning. I was hoping there is a preferred approach for this kind of thing. We're not the first to do it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned during the EU call and the suggestion was to drop the old command in the next release and only support v4 in our next release. Makes it easier to maintain and allows users to use two versions. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the approach is to only support latest IPfabric version we could go this way... but this is a question that we will need to answer more times, and not sure if losing support to "old" ipfabric versions is a good strategy (when there are just some differences).
If we opt for the support for only one version support we have to document it well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I guess it's a trade off between maintenance vs functionality. It's harder and more time-consuming to maintain and test multiple versions, especially in our environment. But it's better for users to have greater support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, it's not the right strategy for function. But it probably is the right strategy for maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go ahead for now, we can implement it later if needed, before deprectaing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a generic validation function for now. I agree, it'll come in handy.