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

feat: add relation fields #22

Merged
merged 12 commits into from
Oct 3, 2019

Conversation

djgilcrease
Copy link
Contributor

feat: add relation fields

  • provides
  • obsoletes
  • suggests
  • recommends
  • requires
  • conflicts

rpm.go Outdated Show resolved Hide resolved
rpm.go Outdated Show resolved Hide resolved
rpm.go Outdated Show resolved Hide resolved
rpm.go Outdated Show resolved Hide resolved
rpm.go Outdated Show resolved Hide resolved
rpm.go Outdated Show resolved Hide resolved
sense_test.go Outdated Show resolved Hide resolved
sense_test.go Outdated Show resolved Hide resolved
@caarlos0
Copy link
Contributor

caarlos0 commented Oct 1, 2019

any news on this?

@jarondl
Copy link
Collaborator

jarondl commented Oct 2, 2019

@caarlos0

any news on this?

Yes, I was on vacation, so I had no time to review it yet. From a quick glance, it feels much better than the earlier version, but the internal API is still clunky. I feel like the AddToIndex function should accept the three tags instead of the "category". I will do a full review tomorrow.

Also, what does sense stand for? Is this a common term for specifying relationships between packages?

sense.go Outdated Show resolved Hide resolved
sense.go Outdated Show resolved Hide resolved
sense.go Show resolved Hide resolved
sense.go Show resolved Hide resolved
sense.go Show resolved Hide resolved
sense.go Show resolved Hide resolved
sense.go Outdated Show resolved Hide resolved
@djgilcrease
Copy link
Contributor Author

Also, what does sense stand for? Is this a common term for specifying relationships between packages?

It is called sense in the rpm c code https://github.com/rpm-software-management/rpm/blob/master/build/parseReqs.c#L81 so I stuck with it

@jarondl
Copy link
Collaborator

jarondl commented Oct 3, 2019

There are still .String() methods. Is this intentional? do you disagree with my comments about their complexities? (That's fine, but let's discuss).

@djgilcrease
Copy link
Contributor Author

djgilcrease commented Oct 3, 2019

There are still .String() methods. Is this intentional? do you disagree with my comments about their complexities? (That's fine, but let's discuss).

$ go build ./cmd/tar2rpm/...
# github.com/google/rpmpack/cmd/tar2rpm
cmd/tar2rpm/main.go:64:11: cannot use &provides (type *rpmpack.Relations) as type flag.Value in argument to flag.Var:
	*rpmpack.Relations does not implement flag.Value (missing String method)

The String function is required to be used as a flag value

@djgilcrease djgilcrease force-pushed the feature/rpm-relationship-sense branch from 2647856 to 230ae50 Compare October 3, 2019 17:40
sense.go Outdated Show resolved Hide resolved
sense.go Outdated Show resolved Hide resolved
@jarondl jarondl merged commit 2f08114 into google:master Oct 3, 2019
@caarlos0
Copy link
Contributor

caarlos0 commented Oct 3, 2019

🚀

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.

4 participants