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

JDF-412 get jdf repo modifications #20

Closed
wants to merge 1 commit into from

Conversation

rafabene
Copy link
Contributor

Before moving JDG to git submodule, we need to get all modifications from JDF repo. This modifications are present in this PR.

@rafabene
Copy link
Contributor Author

Can you provide a tag after merging this PR? Thanks

@mgencur
Copy link
Contributor

mgencur commented Jul 15, 2013

Hi Rafael, I've got a few comments.

  1. Do we really need to include the ASL 2.0 in every single file? For instance, these license headers were recently removed from Infinispan project and there's only one reference to the license in the whole project.
  2. There are a lot of formatting changes, some of them changing indentation from 3 to 4 spaces. Was this intended? In Infinispan/JDG, we use 3-space indentation.
  3. About the change of version to 6.1.0-SNAPSHOT....How about changing it to 6.2.0-SNAPSHOT? The point is that 6.1.0 was already released and when there's a SNAPSHOT in x.y.z-SNAPSHOT version of the project, the next release will be x.y.z, not x.(y+1).z

Thanks
Martin

@rafabene
Copy link
Contributor Author

I'll check 1 and 2 with @pmuir but that is an little answer why it was changed.

  1. I don't know how this applies to Infinispan project. But for JDF it was added after since it is part of contributing guide and QSTools checked for it: https://github.com/jboss-jdf/jboss-as-quickstart/blob/master/CONTRIBUTING.md#license-information-and-contributor-agreement

  2. Same for JDF Contribuiting guide that defines https://github.com/jboss/ide-config as the standard

  3. +1 to use 6.2.0-SNAPSHOT - I'll update the PR after we define 1) and 2)

@pmuir
Copy link

pmuir commented Jul 17, 2013

  1. It's best practice to include license headers in all files, AIUI from Red Hat legal.
  2. 4 spaces is the Java standard, and the JBoss standard, so I think this would be better.
  3. Agreed.

@pmuir
Copy link

pmuir commented Jul 17, 2013

Otherwise looks good.

@mgencur
Copy link
Contributor

mgencur commented Jul 18, 2013

OK. I take you points.

@rafabene
Copy link
Contributor Author

Updated the version to 6.2.0-SNAPSHOT

Thanks

@rafabene
Copy link
Contributor Author

Btw. I'd like to include it on jdf-site 2.1.6 that will be release 07/23.

It should be a great help with you merge and tag it today so I can have this on next release. Thanks

@mgencur
Copy link
Contributor

mgencur commented Jul 22, 2013

Integrated.

@mgencur mgencur closed this Jul 22, 2013
@mgencur
Copy link
Contributor

mgencur commented Jul 22, 2013

Still need to create the tag.

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.

None yet

3 participants