Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update SpdxDependencyInformation.java #145

Closed
wants to merge 3 commits into from
Closed

Update SpdxDependencyInformation.java #145

wants to merge 3 commits into from

Conversation

abhishekdumaniya
Copy link

Type of change

  • Added new project
  • Bug fix
  • New features
  • Enhanced documentation

Changes proposed in this pull request

Checklist

  • I have added relevant comments in my file to make code understandable
  • I have added my project in a folder which includes all the necessary files

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

Thanks @MazeJack for contributing the code.

A couple of comments:

  • I noticed that the code was reformatted. Could you keep the same formatting as the original? The formatting follows the recommended practice for Maven plugins which is different from other Java projects I maintain.
  • Could you write a unit test that demonstrates this is fixed?
  • I also suspect the issue has to do with the code that calls this method, not code in the method itself. The unit tests, however, will tell us if this code works or not.

@abhishekdumaniya
Copy link
Author

I am sorry, I made some blunder so I am closing this PR and creating another one

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