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.

Sign up for GitHub

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

Jump to bottom

Dbld login and image targets #1927

Merged
merged 4 commits into from
Mar 21, 2018
Merged

Dbld login and image targets #1927

merged 4 commits into from
Mar 21, 2018

Conversation

bazsi
Copy link
Collaborator

@bazsi bazsi commented Mar 16, 2018 

dbld/rules login would drop you in a shell within the default container,
just like shell.

dbld/image would just build the default image

@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@kira-syslogng
Copy link
Contributor

success

szemere
szemere suggested changes Mar 18, 2018
View reviewed changes
Copy link
Collaborator

@szemere szemere left a comment

Choose a reason for hiding this comment

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

Earlier, I was thinking about moving every default value to the top. Now they are kinda mixed. What do you think, where should we locate them?

  • To the top, so they will be in one place.
  • Before the -% part, so we can see the relevant defaults in place.

dbld/rules Outdated
shell: shell-xenial
image: image-$(DEFAULT_IMAGE)

shell: shell-$(DEFAULT_IMAGE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes line 87 obsolete. (Like the earlier version too... Ups.)

dbld/rules
@@ -15,6 +15,7 @@ IMAGE_SHELL=balabit/syslog-ng-jessie
VERSION=$(shell cat VERSION)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can You remove line 14? (I can not comment on that line.)

@mitzkia
Copy link
Collaborator

mitzkia commented Mar 18, 2018 •
edited

There are some remaining places where we use xenial as a default image postfix.
For example in rules file:

tarball: tarball-xenial
pkg-tarball: pkg-tarball-xenial
release: deb-xenial

@bazsi
dbld/rules: introduce $(DEFAULT_IMAGE) for specifying which image to …
7b83310 
…work with by default

Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@bazsi
dbld/rules: pull out image list into a dedicated IMAGES variable
f30615d 
Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@bazsi
dbld/rules: add login target
d9678b7 
Signed-off-by: Balazs Scheidler <balazs.scheidler@balabit.com>
@bazsi
dbld/rules: clean up help message
56f9a33
@bazsi bazsi force-pushed the dbld-login-and-image-targets branch from c67ab47 to 56f9a33 Compare March 19, 2018 06:25
@bazsi
Copy link
Collaborator Author

bazsi commented Mar 19, 2018

My latest update should hopefully fix all review notes, and added a few additional cleanups.

@kira-syslogng
Copy link
Contributor

success

@kira-syslogng
Copy link
Contributor

Build SUCCESS, the tests were executed on test branch: master and test suite: functions

@bazsi
Copy link
Collaborator Author

bazsi commented Mar 21, 2018

I would merge this now, as I have two other branches depending on this one, and I think I've addressed the problems with the review notes.

@bazsi bazsi merged commit a54587f into master Mar 21, 2018
@bazsi bazsi removed the in progress label Mar 21, 2018
@MrAnno MrAnno deleted the dbld-login-and-image-targets branch April 15, 2018 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szemere szemere szemere requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bazsi @kira-syslogng @mitzkia @szemere